[U-Boot] [PATCH v2 5/7] sound: tegra: Add a sound driver

[Simon Glass]

Hi Jon,

On Wed, 3 Apr 2019 at 03:15, Jon Hunter <jonathanh at nvidia.com> wrote:
>
>
> On 01/04/2019 21:38, Simon Glass wrote:
> > Add a sound driver for tegra devices. This connects the audio hub, I2S
> > controller and audio codec to allow sound output.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/sound/Makefile      |   2 +-
> >  drivers/sound/tegra_sound.c | 100 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 101 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/sound/tegra_sound.c
> >
> > diff --git a/drivers/sound/Makefile b/drivers/sound/Makefile
> > index 358d5f920b2..73ed7fe53c3 100644
> > --- a/drivers/sound/Makefile
> > +++ b/drivers/sound/Makefile
> > @@ -11,7 +11,7 @@ obj-$(CONFIG_I2S_SAMSUNG)   += samsung-i2s.o
> >  obj-$(CONFIG_SOUND_SANDBOX)  += sandbox.o
> >  obj-$(CONFIG_I2S_ROCKCHIP)   += rockchip_i2s.o rockchip_sound.o
> >  obj-$(CONFIG_I2S_SAMSUNG)    += samsung_sound.o
> > -obj-$(CONFIG_I2S_TEGRA)              += tegra_ahub.o tegra_i2s.o
> > +obj-$(CONFIG_I2S_TEGRA)              += tegra_ahub.o tegra_i2s.o tegra_sound.o
> >  obj-$(CONFIG_SOUND_WM8994)   += wm8994.o
> >  obj-$(CONFIG_SOUND_MAX98088) += max98088.o maxim_codec.o
> >  obj-$(CONFIG_SOUND_MAX98090) += max98090.o maxim_codec.o
> > diff --git a/drivers/sound/tegra_sound.c b/drivers/sound/tegra_sound.c
> > new file mode 100644
> > index 00000000000..7c2ed53f5a7
> > --- /dev/null
> > +++ b/drivers/sound/tegra_sound.c
> > @@ -0,0 +1,100 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2018 Google, LLC
> > + * Written by Simon Glass <sjg at chromium.org>
> > + */
> > +
> > +#define LOG_CATEGORY UCLASS_I2S
> > +
> > +#include <common.h>
> > +#include <audio_codec.h>
> > +#include <dm.h>
> > +#include <i2s.h>
> > +#include <misc.h>
> > +#include <sound.h>
> > +#include <asm/gpio.h>
> > +#include "tegra_i2s_priv.h"
> > +
> > +static int tegra_sound_setup(struct udevice *dev)
> > +{
> > +     struct sound_uc_priv *uc_priv = dev_get_uclass_priv(dev);
> > +     struct i2s_uc_priv *i2c_priv = dev_get_uclass_priv(uc_priv->i2s);
> > +     int ret;
> > +
> > +     if (uc_priv->setup_done)
> > +             return -EALREADY;
> > +     ret = audio_codec_set_params(uc_priv->codec, i2c_priv->id,
> > +                                  i2c_priv->samplingrate,
> > +                                  i2c_priv->samplingrate * i2c_priv->rfs,
> > +                                  i2c_priv->bitspersample,
> > +                                  i2c_priv->channels);
> > +     if (ret)
> > +             return ret;
> > +     uc_priv->setup_done = true;
> > +
> > +     return 0;
> > +}
> > +
> > +static int tegra_sound_play(struct udevice *dev, void *data, uint data_size)
> > +{
> > +     struct sound_uc_priv *uc_priv = dev_get_uclass_priv(dev);
> > +
> > +     return i2s_tx_data(uc_priv->i2s, data, data_size);
> > +}
> > +
> > +static int tegra_sound_probe(struct udevice *dev)
> > +{
> > +     struct sound_uc_priv *uc_priv = dev_get_uclass_priv(dev);
> > +     struct gpio_desc en_gpio;
> > +     struct udevice *ahub;
> > +     int ret;
> > +
> > +     ret = gpio_request_by_name(dev, "codec-enable-gpio", 0, &en_gpio,
> > +                                GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
>
>
> This looks a bit odd. I am not sure if this is supposed to be optional
> or not, but there is no checking of the return value although you store it.

Yes. If there is an error then en_gpio will not be set up. Then later
on we won't set the GPIO when enabling the codec.

>
> Is there DT binding documentation that does along with this that
> describes these properties?

Unfortunately not. The binding is actually board-specific in many
cases. I'll do a little series to in these files for all boards
including nyan.

Regards,
Simon