Received-SPF: pass (google.com: domain of linux-kernel+bounces-210039-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <ca44b61a-3399-41b4-9f78-81a2ae963ce3@collabora.com>
Date: Tue, 11 Jun 2024 16:09:31 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1] ASoC: support machine driver with TAS2781
To: Mark Brown <broonie@kernel.org>
Cc: Rui Zhou <zhourui@huaqin.corp-partner.google.com>, lgirdwood@gmail.com,
 perex@perex.cz, tiwai@suse.com, matthias.bgg@gmail.com,
 trevor.wu@mediatek.com, allen-kh.cheng@mediatek.com,
 kuninori.morimoto.gx@renesas.com, maso.huang@mediatek.com,
 linux-sound@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
 linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org
References: <20240611022227.2508354-1-zhourui@huaqin.corp-partner.google.com>
 <Zmgsfafmfedd9_MV@finisterre.sirena.org.uk>
 <d7d32ef3-8b2e-476c-a15e-7f829ea0d60a@collabora.com>
 <ZmhOz7BB8eJ38QVE@finisterre.sirena.org.uk>
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Content-Language: en-US
In-Reply-To: <ZmhOz7BB8eJ38QVE@finisterre.sirena.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Il 11/06/24 15:19, Mark Brown ha scritto:
> On Tue, Jun 11, 2024 at 02:38:09PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 11/06/24 12:52, Mark Brown ha scritto:
>>> On Tue, Jun 11, 2024 at 10:22:27AM +0800, Rui Zhou wrote:
>>>> Support amp TAS2781 for mt8188 platform
> 
>>>> +++ b/sound/soc/mediatek/Kconfig
>>>> @@ -233,6 +233,7 @@ config SND_SOC_MT8188_MT6359
>>>>    	select SND_SOC_HDMI_CODEC
>>>>    	select SND_SOC_DMIC
>>>>    	select SND_SOC_MAX98390
>>>> +	select SND_SOC_TAS2781_I2C
>>>>    	select SND_SOC_NAU8315
>>>>    	select SND_SOC_NAU8825
> 
>>> Ideally this would be sorted to avoid spurious merge conficts.  Also do
>>> we really have dependencies here or should these just be being enabled
>>> by users - these drivers are all pretty generic now?
> 
>> The only dependency is on the MT6359 PMIC's audio related IPs and nothing else.
> 
>> I'd even remove the other select statements for the other codecs, honestly,
>> as there's no reason why MT8188 wouldn't support a codec or the other, and
>> there is no reason why that SoC needs exactly those codecs, apart from some
>> board (Chromebooks) having one, or the other.
> 
> Right, that's what I meant - the drivers are now so generic that they
> probably shouldn't be selecting everything they could possibly work
> with.
> 
>> Instead of adding a select statement on MT8188_MT6359, adding the TAS2781_I2C
>> driver as a module in defconfig is a better idea..... at least, IMO.
> 
> Yes.
> 
>>>   Not that this
>>> change in itself makes a huge difference to the existing state.
> 
>> I agree it doesn't make any huge difference, but I also don't see why users
>> can't simply enable (y/m) that in the kernel configuration :-)
> 
> I think this is a result of the drivers originally being very specific
> to a particular setup

Yeah, I agree.

 >
> and gradually getting made more generic, if the
> driver can only be used on a specific machine then the selects make
> sense but that's no longer the case for these drivers I think.

No this is not going to be used only for a specific machine... so yeah your
thoughts are totally right.

Just as an example, MT8188 (Kompanio) and MT8390 (Genio) are the same SoC again,
like MT8195 (Kompanio) and MT8395 (Genio) - so this driver is also going to be
used for the Genio 700 EVK, other than some SBC, etc, which do anyway expose I2S
pins on some kind of header, so... :-)

Cheers!