lkml.org 
[lkml]   [2020]   [Oct]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] ASoC: tas2764: Add the driver for the TAS2764
On Wed, Sep 30, 2020 at 11:38:09AM -0500, Dan Murphy wrote:

This all looks good - a few very minor things below but nothing
substantial:

> + default:
> + dev_err(tas2764->dev, "Not supported evevt\n");
> + return -EINVAL;

evevt -> event

> +static int tas2764_mute(struct snd_soc_dai *dai, int mute, int direction)
> +{
> + struct snd_soc_component *component = dai->component;
> + int ret = snd_soc_component_update_bits(component, TAS2764_PWR_CTRL,
> + TAS2764_PWR_CTRL_MASK,
> + mute ? TAS2764_PWR_CTRL_MUTE : 0);
> +
> + if (ret < 0)
> + return ret;

This looks weird with the ternery operator and extreme indentation -
could you please at least split the declaration of ret from the call to
make the line length a bit extreme?

> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + case SND_SOC_DAIFMT_DSP_A:
> + tdm_rx_start_slot = 1;
> + break;
> + case SND_SOC_DAIFMT_DSP_B:
> + case SND_SOC_DAIFMT_LEFT_J:
> + tdm_rx_start_slot = 0;
> + break;

I'm not seeing any other handling that distinguishes between the I2S and
DSP modes anywhere - I'm guessing this is because the device is really
only implementing the DSP modes but because it's mono this is compatible
with the I2S modes? It'd be worth having a comment saying this since
while that would be OK not distinguishing between modes properly is a
common error in drivers so it'd help avoid cut'n'paste issues if someone
uses this code as a reference.

> +static int tas2764_register_codec(struct tas2764_priv *tas2764)
> +{
> + return devm_snd_soc_register_component(tas2764->dev,
> + &soc_component_driver_tas2764,
> + tas2764_dai_driver,
> + ARRAY_SIZE(tas2764_dai_driver));
> +}

This is a bit odd - can we not just inline the component registration
rather than having this function?
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-10-01 18:26    [W:0.110 / U:2.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site