| Message ID | 1611920470-24225-5-git-send-email-spujar@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Tegra186 and Tegra194 audio graph card | expand |
On Fri, Jan 29, 2021 at 05:11:06PM +0530, Sameer Pujar wrote: > Select SND_SOC_RT5659 to verify external audio over Jetson platforms. > Jetson AGX Xavier has an on-board RT5658 audio codec and to use this > enable required config. > > config SND_SOC_TEGRA_AUDIO_GRAPH_CARD > tristate "Audio Graph Card based Tegra driver" > depends on SND_AUDIO_GRAPH_CARD > + select SND_SOC_RT5659 > help This is a generic device, not something specific to Jetson, why force on a driver that may not be required on other boards using this driver?
On 1/29/2021 6:14 PM, Mark Brown wrote: > On Fri, Jan 29, 2021 at 05:11:06PM +0530, Sameer Pujar wrote: >> Select SND_SOC_RT5659 to verify external audio over Jetson platforms. >> Jetson AGX Xavier has an on-board RT5658 audio codec and to use this >> enable required config. >> >> config SND_SOC_TEGRA_AUDIO_GRAPH_CARD >> tristate "Audio Graph Card based Tegra driver" >> depends on SND_AUDIO_GRAPH_CARD >> + select SND_SOC_RT5659 >> help > This is a generic device, not something specific to Jetson, why force > on a driver that may not be required on other boards using this driver? Yes specific boards using 'SND_SOC_TEGRA_AUDIO_GRAPH_CARD' may require 'SND_SOC_RT5659'. But there is no platform specific config. Is there a better way to enable config 'SND_SOC_RT5659' conditionally?
On Fri, Jan 29, 2021 at 09:02:52PM +0530, Sameer Pujar wrote: > On 1/29/2021 6:14 PM, Mark Brown wrote: > > This is a generic device, not something specific to Jetson, why force > > on a driver that may not be required on other boards using this driver? > Yes specific boards using 'SND_SOC_TEGRA_AUDIO_GRAPH_CARD' may require > 'SND_SOC_RT5659'. But there is no platform specific config. Is there a > better way to enable config 'SND_SOC_RT5659' conditionally? If the user wants a given CODEC driver then they should enable that driver.
On 1/29/2021 9:18 PM, Mark Brown wrote: > On Fri, Jan 29, 2021 at 09:02:52PM +0530, Sameer Pujar wrote: >> On 1/29/2021 6:14 PM, Mark Brown wrote: >>> This is a generic device, not something specific to Jetson, why force >>> on a driver that may not be required on other boards using this driver? >> Yes specific boards using 'SND_SOC_TEGRA_AUDIO_GRAPH_CARD' may require >> 'SND_SOC_RT5659'. But there is no platform specific config. Is there a >> better way to enable config 'SND_SOC_RT5659' conditionally? > If the user wants a given CODEC driver then they should enable that > driver. Above card driver is intended to be used on multiple platforms. DT has already a way for user to describe the CODEC connection required for specific platform. So idea is enable these CODECs from driver point of view for this card driver and platform DT can use what is required. Also the CODEC driver will be a loadable module here. If above does not seem fine, alternatively can I just enable CODEC config independently from defconfig?
On Fri, Jan 29, 2021 at 10:15:51PM +0530, Sameer Pujar wrote: > On 1/29/2021 9:18 PM, Mark Brown wrote: > > If the user wants a given CODEC driver then they should enable that > > driver. > Above card driver is intended to be used on multiple platforms. DT has > already a way for user to describe the CODEC connection required for > specific platform. So idea is enable these CODECs from driver point of view > for this card driver and platform DT can use what is required. Also the > CODEC driver will be a loadable module here. No, you're missing the point of a generic driver here. This will mean that if someone wants to build the driver into the kernel for their embedded board they will be forced to build in every CODEC driver someone has decided might be used with this generic driver, and even if they're building things modular someone trying to cut down the size of their kernel images is going to at least have to spend time building CODEC drivers they don't want to use. Distributions should just select all the CODEC drivers that are available in Kconfig, people configuring for a particular target audience should be able to build only the drivers they know they need. > If above does not seem fine, alternatively can I just enable CODEC config > independently from defconfig? Yes.
On 1/29/2021 10:49 PM, Mark Brown wrote: > On Fri, Jan 29, 2021 at 10:15:51PM +0530, Sameer Pujar wrote: >> On 1/29/2021 9:18 PM, Mark Brown wrote: >>> If the user wants a given CODEC driver then they should enable that >>> driver. >> Above card driver is intended to be used on multiple platforms. DT has >> already a way for user to describe the CODEC connection required for >> specific platform. So idea is enable these CODECs from driver point of view >> for this card driver and platform DT can use what is required. Also the >> CODEC driver will be a loadable module here. > No, you're missing the point of a generic driver here. This will mean > that if someone wants to build the driver into the kernel for their > embedded board they will be forced to build in every CODEC driver > someone has decided might be used with this generic driver, and even if > they're building things modular someone trying to cut down the size of > their kernel images is going to at least have to spend time building > CODEC drivers they don't want to use. Distributions should just select > all the CODEC drivers that are available in Kconfig, people configuring > for a particular target audience should be able to build only the > drivers they know they need. OK, will drop this in v2. Thank you for details. > >> If above does not seem fine, alternatively can I just enable CODEC config >> independently from defconfig? > Yes.
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig index 6dc83ad..4a45163 100644 --- a/sound/soc/tegra/Kconfig +++ b/sound/soc/tegra/Kconfig @@ -120,6 +120,7 @@ config SND_SOC_TEGRA210_ADMAIF config SND_SOC_TEGRA_AUDIO_GRAPH_CARD tristate "Audio Graph Card based Tegra driver" depends on SND_AUDIO_GRAPH_CARD + select SND_SOC_RT5659 help Config to enable Tegra audio machine driver based on generic audio graph driver. It is a thin driver written to customize