Re: [EXTERNAL] Re: [PATCH 1/6] sound: ti: Add sound support for am625 board in Uboot

[Scaria Kochidanadu <s-kochidanadu@ti.com>]

On 08/07/24 21:16, Tom Rini wrote:
> On Mon, Jul 08, 2024 at 01:22:02PM +0530, Scaria Kochidanadu wrote:
> 
>> Add a UCLASS_SOUND driver for Texas Instruments SoCs which ties together
>> the tlv320aic3106 audio codec and MCASP I2S controller. Enable audio
>> playback functionality by taking a data pointer and data size as the
>> sound data. The uboot sound play command takes time and frequency as
>> input and creates the data for a beep sound with the given parameters,
>> which is then passed to the sound play function.
> [snip]
>> +++ b/drivers/sound/ti_sound.c
>> @@ -0,0 +1,119 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2024 Texas Instruments Incorporated - https://www.ti.com/
>> + * Scaria M Kochidanadu, s-kochidanadu@ti.com
>> + *
>> + * based on the uboot samsung sound driver, which is
>> + *
>> + * Copyright 2018 Google, LLC
>> + * Written by Simon Glass <sjg@chromium.org>
>> + */
>> +
>> +#include <asm/u-boot.h> /* boot information for Linux kernel */
>> +/* Pull in stuff for the build system */
>> +#ifdef DO_DEPS_ONLY
>> +#include <env_internal.h>
>> +#endif
>> +#include <audio_codec.h>
>> +#include <dm.h>
>> +#include <i2s.h>
>> +#include <log.h>
>> +#include <sound.h>
>> +#include <asm/gpio.h>
> 
> Please audit this list. I really am confused by <env_internal.h>.
> 

I will be removing this in v2 as it is not required.

>> +static int ti_sound_stop_play(struct udevice *dev)
>> +{
>> +	/* This function is necassary to satisfy the function calls
>> +	 * in the Uboot command: sound play
>> +	 */
>> +	return 0;
> 
> /*
>  * Like this...
>  */
> 
> [snip]
>> +static const struct udevice_id ti_sound_ids[] = {
>> +	{ .compatible = "simple-audio-card" },
>> +	{ }
>> +};
> 
> Adding an actual simple-audio-card framework for U-Boot would be great.
> This driver implies (and is?) TI-centric, instead. So lets rework this
> to be split appropriately? Thanks.
> 

I wanted to reuse the sound card DT node in the upstream kernel
device-tree. The U-boot upstrem repo does not have the node as it is
using an older version of DT.
But if using the simple-audio-card node is not preferred, then can we
create TI specific compatible and a separate DT node?