Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Limonciello, Mario" <Mario.Limonciello@dell.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Mark Brown <broonie@kernel.org>,
        "Yuan, Perry" <Perry.Yuan@dell.com>
CC:     "oder_chiou@realtek.com" <oder_chiou@realtek.com>,
        "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
        "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tiwai@suse.com" <tiwai@suse.com>
Subject: RE: [PATCH] ASoC: rt715:add Mic Mute LED control support
Thread-Topic: [PATCH] ASoC: rt715:add Mic Mute LED control support
Thread-Index: AQHWseEjYgOvfJhc/UebM8naQzkVCam2YlCAgAAyV4CAABsq0A==
Date:   Tue, 3 Nov 2020 17:51:57 +0000
Message-ID: <DM6PR19MB2636291E083E1843B0E0035CFA110@DM6PR19MB2636.namprd19.prod.outlook.com>
References: <20201103125859.8759-1-Perry_Yuan@Dell.com>
 <20201103131253.GA5545@sirena.org.uk>
 <5f2c1282-4401-276a-8dad-127fa1f449fd@linux.intel.com>
In-Reply-To: <5f2c1282-4401-276a-8dad-127fa1f449fd@linux.intel.com>
Accept-Language: en-US
Content-Language: en-US
msip_labels: MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Enabled=True;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_SiteId=945c199a-83a2-4e80-9f8c-5a91be5752dd;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Owner=Mario_Limonciello@Dell.com;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_SetDate=2020-11-03T17:51:51.7023762Z;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Name=External Public;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Application=Microsoft Azure
 Information Protection;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_ActionId=ba2e31ec-2f80-405d-b00a-337bb358e52d;
 MSIP_Label_17cb76b2-10b8-4fe1-93d4-2202842406cd_Extended_MSFT_Method=Manual
authentication-results: linux.intel.com; dkim=none (message not signed)
 header.d=none;linux.intel.com; dmarc=none action=none header.from=Dell.com;
x-originating-ip: [76.251.167.31]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: da416816-ab46-4b0d-c50e-08d880212884
x-ms-traffictypediagnostic: DM5PR19MB0923:
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <DM5PR19MB092348F262602E9AD6004EFDFA110@DM5PR19MB0923.namprd19.prod.outlook.com>
x-exotenant: 2khUwGVqB6N9v58KS13ncyUmMJd8q4
x-ms-oob-tlc-oobclassifiers: OLM:8882;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: MsgHMR5rxVW40BDXiVWUkHfbF6Dj2D2UE2+bcs006+2TBRQ689BOHzT/RDM/bkRbrAzSOH2bllnWN2dVh/JWyxjTa25Z9bwihUOwPqdpMBiPQCXAZJI4qe+YEfa9CvUGuSXwsum4Nl2EeR8nfoDd7AMLuwQR5Jyd1r1ognaeYXtfxj+oMW6SW2E4m15VAd1bmDyBYKJZwtX9zN8+UV6zDCXQ83YYffH/ldk6Nuad6oco4YpmQqUT5kDTCt4lchWHQpDmxCg7MoQrYx/uVscM0VW5PvMWMQrJ8BAN2/pYmtRoQGEclCofBijY2ZGPO/aNWiLEVUnoNRfoUhhcXkAThihKMwd9+f4YGIx8ffVeUZ80Vu5AoVaFjAXRluvTf16GB0us99Jfi6gPLD8NsW3R7g==
x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM6PR19MB2636.namprd19.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(396003)(136003)(39860400002)(376002)(346002)(53546011)(83380400001)(33656002)(55016002)(2906002)(6506007)(4326008)(478600001)(186003)(8676002)(7696005)(8936002)(71200400001)(66476007)(66556008)(6636002)(76116006)(66946007)(110136005)(9686003)(316002)(86362001)(26005)(786003)(5660300002)(966005)(64756008)(66446008)(52536014)(54906003);DIR:OUT;SFP:1101;
x-ms-exchange-antispam-messagedata: ojav0AAlpeoWezU9R0qIkkHMaUYFa68ue40t7mVFyaXQJs0jK/EP96Kxt9oq5BZSOyn4wze+FWtOunXGHY/yEflbdpYEepKgSFe7IKQ8aYRIhHkPBTXze29OQyxVWbl4Xzj6SS/PB++V2dnGkAe80AFfrcdIys5H6x/ft3AHVH+Ybjfv/OC0StKF3yFVrzhJmbto3OmJpgQjVr3M+NaiHxgUE2n6i6+wHh7Rxrh5Z9h5vUi7LAVHMp8Ol2Jqi/0hCBw5D4hJpEuBR9JNcA67CN89/c4qngK6HyGcqTo1Kxc9W2QeoOzYrtbVV2TuydA/xytGPSTfkrZh3TXof+Om56R42PdvSY/OxPd5BwAaslB5HpI55Qvi0Co66vmCjSDEFOXv5Av8vx2D1iqc8MqiCZNBDR7psNqpayy59GK2FyT0PxBqMtLHy2JQV7rhi7EYJyoh0q7ecUiXdey1ICWE8wbUJmeMxDnucItvXaYvI7I604C6hEVpd22LI2Xd6vaQ+oVxYDq0TO3w9qPJQZJud7ZS/frlnDUR2UK545dBoJwBuHxd1bHG4XWbuLkOxLpzrqWR307LiiYoyWaF9SGUv9F/EPpWl0pDl/PqpJtyOH2mofuPV9uzU8X61NbJq6Dll2L7mlEMDsI6GHwc71L8nA==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Dell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR19MB2636.namprd19.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: da416816-ab46-4b0d-c50e-08d880212884
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Nov 2020 17:51:57.3461
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 945c199a-83a2-4e80-9f8c-5a91be5752dd
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: WavswQe1Sx2rOkNO2qHeniHcXQk5jQKy7U2Aer+vjODESIfoF8t48KzJYznzphvv7EkaOZ6SEvnlxn4dJDN14f8OECKmSl++UZ3iXan4r/s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR19MB0923
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312,18.0.737
 definitions=2020-11-03_08:2020-11-03,2020-11-03 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 phishscore=0 clxscore=1011
 priorityscore=1501 mlxscore=0 bulkscore=0 impostorscore=0 malwarescore=0
 suspectscore=0 spamscore=0 lowpriorityscore=0 adultscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2011030120
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxlogscore=999 bulkscore=0 phishscore=0 mlxscore=0 spamscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2011030120
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Sent: Tuesday, November 3, 2020 10:13
> To: Mark Brown; Yuan, Perry
> Cc: oder_chiou@realtek.com; alsa-devel@alsa-project.org; lgirdwood@gmail.=
com;
> Limonciello, Mario; linux-kernel@vger.kernel.org; tiwai@suse.com
> Subject: Re: [PATCH] ASoC: rt715:add Mic Mute LED control support
>=20
>=20
> [EXTERNAL EMAIL]
>=20
> Somehow this patch was filtered by alsa-devel servers?
>=20
> On 11/3/20 7:12 AM, Mark Brown wrote:
> > On Tue, Nov 03, 2020 at 04:58:59AM -0800, Perry Yuan wrote:
> >> From: perry_yuan <perry_yuan@dell.com>
> >>
> >> Some new Dell system is going to support audio internal micphone
> >> privacy setting from hardware level with micmute led state changing
> >>
> >> This patch allow to change micmute led state through this micphone
> >> led control interface like hda_generic provided.
> >
> > If this is useful it should be done at the subsystem level rather than
> > open coded in a specific CODEC driver, however I don't undersand why it
> > is.
> >
> >> +static int rt715_micmute_led_mode_put(struct snd_kcontrol *kcontrol,
> >> +      struct snd_ctl_elem_value *ucontrol)
> >> +{
> >> +    struct snd_soc_component *component =3D snd_kcontrol_chip(kcontro=
l);
> >> +    struct rt715_priv *rt715 =3D snd_soc_component_get_drvdata(compon=
ent);
> >> +    int led_mode =3D ucontrol->value.integer.value[0];
> >> +
> >> +    rt715->micmute_led =3D led_mode;
> >> +#if IS_ENABLED(CONFIG_LEDS_TRIGGER_AUDIO)
> >> +    ledtrig_audio_set(LED_AUDIO_MICMUTE,
> >> +               rt715->micmute_led ? LED_ON : LED_OFF);
> >> +#endif
> >> +  return 0;
> >> +}
> >
> > This is just adding a userspace API to set a LED via the standard LED
> > APIs.  Since the LED subsystem already has a perfectly good userspace
> > API why not use that?  There is no visible value in this being in the
> > sound subsystem.
>=20
> I also don't quite follow. This looks as inspired from HDaudio code, but
> with a lot of simplifications.
>=20
> If the intent was that when userspace decides to mute the LED is turned
> on, wouldn't it be enough to just track the state of a 'capture switch'
> responsible for mute, i.e. when the capture Switch is 'off' the LED is
> on. I don't see the point of having a new control, you would be adding
> more work for PulseAudio/UCM whereas connecting the capture switch to a
> led comes with zero work in userspace. See e.g. how the mute mic LED was
> handled in the SOF code handling DMICs, we didn't add a new control but
> turned the LED in the switch .put callback, see
>=20
> https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L1=
8
>=20
> https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/control.c#L1=
53
>=20
> Actually thinking more about it, having two controls for 'mute LED' and
> 'capture switch' could lead to inconsistent states where the LED is on
> without mute being activated. we should really bolt the LED activation
> to the capture switch, that way the mute and LED states are aligned.
>=20

After giving it some thought I agree.  The UCM change that was opened
here https://github.com/alsa-project/alsa-ucm-conf/pull/60 wouldn't
be necessary at all if you just track capture switch directly like SOF does=
.