Messages in this thread | | | Subject | Re: [PATCH v2 3/3] ASoC: tlv320adcx140: Add proper support for master mode | From | Dan Murphy <> | Date | Tue, 15 Sep 2020 13:50:43 -0500 |
| |
Camel
On 9/11/20 3:07 AM, Camel Guo wrote: > From: Camel Guo <camelg@axis.com> > > Add setup of bclk-to-ws ratio and sample rate when in master mode, > as well as MCLK input pin setup. > > Signed-off-by: Camel Guo <camelg@axis.com> > --- > v2: > - Move GPIO setting into devicetree > - Move master config register setting into a new function > > sound/soc/codecs/tlv320adcx140.c | 139 ++++++++++++++++++++++++++++++- > sound/soc/codecs/tlv320adcx140.h | 27 ++++++ > 2 files changed, 162 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/codecs/tlv320adcx140.c b/sound/soc/codecs/tlv320adcx140.c > index 97f16fbba441..685f5fd8b537 100644 > --- a/sound/soc/codecs/tlv320adcx140.c > +++ b/sound/soc/codecs/tlv320adcx140.c > @@ -35,6 +35,7 @@ struct adcx140_priv { > unsigned int dai_fmt; > unsigned int tdm_delay; > unsigned int slot_width; > + bool master; > }; > > static const char * const gpo_config_names[] = { > @@ -651,11 +652,136 @@ static int adcx140_reset(struct adcx140_priv *adcx140) > return ret; > } > > +static int adcx140_fs_bclk_ratio(unsigned int bclk_ratio) > +{ > + switch (bclk_ratio) { > + case 16: > + return ADCX140_RATIO_16; > + case 24: > + return ADCX140_RATIO_24; > + case 32: > + return ADCX140_RATIO_32; > + case 48: > + return ADCX140_RATIO_48; > + case 64: > + return ADCX140_RATIO_64; > + case 96: > + return ADCX140_RATIO_96; > + case 128: > + return ADCX140_RATIO_128; > + case 192: > + return ADCX140_RATIO_192; > + case 256: > + return ADCX140_RATIO_256; > + case 384: > + return ADCX140_RATIO_384; > + case 512: > + return ADCX140_RATIO_512; > + case 1024: > + return ADCX140_RATIO_1024; > + case 2048: > + return ADCX140_RATIO_2048; > + default: > + break; > + } > + return -EINVAL; > +} > + > +static int adcx140_fs_rate(unsigned int rate) > +{ > + switch (rate) { > + case 7350: > + case 8000: > + return ADCX140_8_OR_7_35KHZ; > + case 14700: > + case 16000: > + return ADCX140_16_OR_14_7KHZ; > + case 22050: > + case 24000: > + return ADCX140_24_OR_22_05KHZ; > + case 29400: > + case 32000: > + return ADCX140_32_OR_29_4KHZ; > + case 44100: > + case 48000: > + return ADCX140_48_OR_44_1KHZ; > + case 88200: > + case 96000: > + return ADCX140_96_OR_88_2KHZ; > + case 176400: > + case 192000: > + return ADCX140_192_OR_176_4KHZ; > + case 352800: > + case 384000: > + return ADCX140_384_OR_352_8KHZ; > + case 705600: > + case 768000: > + return ADCX140_768_OR_705_6KHZ; > + default: > + break; > + } > + return -EINVAL; > +} > + > +static int adcx140_setup_master_config(struct snd_soc_component *component, > + struct snd_pcm_hw_params *params) > +{ > + int ret = 0; > + struct adcx140_priv *adcx140 = snd_soc_component_get_drvdata(component); > + > + if (adcx140->master) {
Move this out to hw_params. No reason to jump here just to jump back.
Check for master and if master then configure
This will eliminate the mixed code and variable declaration below which is what I wanted to avoid in v1.
This will also allow you to remove some of the variable initialization.
> + u8 mst_cfg1 = 0; > + u8 mst_cfg0 = 0; This can be init to mst_cfg0 = ADCX140_BCLK_FSYNC_MASTER no reason to set it here and then change it immediately. > + unsigned int bclk_ratio; > + > + mst_cfg0 = ADCX140_BCLK_FSYNC_MASTER; > + if (params_rate(params) % 1000) > + mst_cfg0 |= ADCX140_FSYNCINV_BIT; /* 44.1 kHz et al */ > + > + ret = adcx140_fs_rate(params_rate(params)); > + if (ret < 0) { > + dev_err(adcx140->dev, "%s: Unsupported rate %d\n", > + __func__, params_rate(params)); > + return ret; > + } > + mst_cfg1 |= ret; Why the | here? This is initialized to 0 so mst_cfg1 = ret. And why even use ret just return into mst_cfg1 and check that variable > + > + /* In slave mode when using automatic clock configuration, > + * the codec figures out the BCLK to FSYNC ratio itself. But > + * here in master mode, we need to tell it. > + */ > + > + bclk_ratio = snd_soc_params_to_frame_size(params); > + ret = adcx140_fs_bclk_ratio(bclk_ratio); > + if (ret < 0) { > + dev_err(adcx140->dev, "%s: Unsupported bclk_ratio %d\n", > + __func__, bclk_ratio); > + return ret; > + } > + mst_cfg1 |= ret; > + > + snd_soc_component_update_bits(component, ADCX140_MST_CFG1, > + ADCX140_FS_RATE_MSK | > + ADCX140_RATIO_MSK, > + mst_cfg1);
I don't understand the update_bits since you have calcualted both the Ratio and rate you can just write the register with the snd_soc_component_write.
> + > + snd_soc_component_update_bits(component, ADCX140_MST_CFG0, > + ADCX140_FSYNCINV_BIT | > + ADCX140_BCLK_FSYNC_MASTER, > + mst_cfg0); > +
But this is ok. I actually have other changes I am posting which move this to the set_dai_format. So I am not sure if this will be needed after that patch is applied.
I will CC you on those patches.
Dan
| |