Message ID | 1580380422-3431-3-git-send-email-spujar@nvidia.com |
---|---|
State | Changes Requested |
Headers | show |
Series | add ASoC components for AHUB | expand |
30.01.2020 13:33, Sameer Pujar пишет: ... > +#include <linux/module.h> > +#include <linux/regmap.h> > +#include "tegra_cif.h" > + > +void tegra_set_cif(struct regmap *regmap, unsigned int reg, > + struct tegra_cif_conf *conf) > +{ > + unsigned int value; > + > + value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) | > + ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) | > + ((conf->client_ch - 1) << TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) | > + (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) | > + (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) | > + (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) | > + (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) | > + (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) | > + (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) | > + (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT); > + > + regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value); > +} > +EXPORT_SYMBOL_GPL(tegra_set_cif); Are you going to add more stuff into this source file later on? If not, then it's too much to have a separate driver module just for a single tiny function, you should move it into the header file (make it inline).
On 2/5/2020 10:32 PM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 30.01.2020 13:33, Sameer Pujar пишет: > ... >> +#include <linux/module.h> >> +#include <linux/regmap.h> >> +#include "tegra_cif.h" >> + >> +void tegra_set_cif(struct regmap *regmap, unsigned int reg, >> + struct tegra_cif_conf *conf) >> +{ >> + unsigned int value; >> + >> + value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) | >> + ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) | >> + ((conf->client_ch - 1) << TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) | >> + (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) | >> + (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) | >> + (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) | >> + (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) | >> + (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) | >> + (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) | >> + (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT); >> + >> + regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value); >> +} >> +EXPORT_SYMBOL_GPL(tegra_set_cif); > Are you going to add more stuff into this source file later on? Yes I plan to add Tegra30 and Tegra124 CIF functions in this. Anything related to CIF can be moved here. > > If not, then it's too much to have a separate driver module just for a > single tiny function, you should move it into the header file (make it > inline).
06.02.2020 14:56, Sameer Pujar пишет: > > > On 2/5/2020 10:32 PM, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 30.01.2020 13:33, Sameer Pujar пишет: >> ... >>> +#include <linux/module.h> >>> +#include <linux/regmap.h> >>> +#include "tegra_cif.h" >>> + >>> +void tegra_set_cif(struct regmap *regmap, unsigned int reg, >>> + struct tegra_cif_conf *conf) >>> +{ >>> + unsigned int value; >>> + >>> + value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) | >>> + ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) | >>> + ((conf->client_ch - 1) << >>> TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) | >>> + (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) | >>> + (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) | >>> + (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) | >>> + (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) | >>> + (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) | >>> + (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) | >>> + (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT); >>> + >>> + regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value); >>> +} >>> +EXPORT_SYMBOL_GPL(tegra_set_cif); >> Are you going to add more stuff into this source file later on? > > Yes I plan to add Tegra30 and Tegra124 CIF functions in this. Anything > related to CIF can be moved here. >> >> If not, then it's too much to have a separate driver module just for a >> single tiny function, you should move it into the header file (make it >> inline). > You should consider whether it's worth to move anything else to this module first, because if the functions are not reusable by the drivers, then the movement won't bring any benefits and final result could be negative in regards to the code's organization. I suggest to start clean and easy, without the driver module. You will be able to factor code into module later on, once there will a real need to do that.
On 2/6/2020 10:19 PM, Dmitry Osipenko wrote: > External email: Use caution opening links or attachments > > > 06.02.2020 14:56, Sameer Pujar пишет: >> >> On 2/5/2020 10:32 PM, Dmitry Osipenko wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> 30.01.2020 13:33, Sameer Pujar пишет: >>> ... >>>> +#include <linux/module.h> >>>> +#include <linux/regmap.h> >>>> +#include "tegra_cif.h" >>>> + >>>> +void tegra_set_cif(struct regmap *regmap, unsigned int reg, >>>> + struct tegra_cif_conf *conf) >>>> +{ >>>> + unsigned int value; >>>> + >>>> + value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) | >>>> + ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) | >>>> + ((conf->client_ch - 1) << >>>> TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) | >>>> + (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) | >>>> + (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) | >>>> + (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) | >>>> + (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) | >>>> + (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) | >>>> + (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) | >>>> + (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT); >>>> + >>>> + regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value); >>>> +} >>>> +EXPORT_SYMBOL_GPL(tegra_set_cif); >>> Are you going to add more stuff into this source file later on? >> Yes I plan to add Tegra30 and Tegra124 CIF functions in this. Anything >> related to CIF can be moved here. >>> If not, then it's too much to have a separate driver module just for a >>> single tiny function, you should move it into the header file (make it >>> inline). > You should consider whether it's worth to move anything else to this > module first, because if the functions are not reusable by the drivers, > then the movement won't bring any benefits and final result could be > negative in regards to the code's organization. > > I suggest to start clean and easy, without the driver module. You will > be able to factor code into module later on, once there will a real need > to do that. Tegra124 can reuse above CIF function. Tegra30 will continue to use the same function. For consistency all CIF related helpers can be grouped at one place. But this is for later. I will start with inline function.
diff --git a/sound/soc/tegra/Makefile b/sound/soc/tegra/Makefile index c84f183..261aa21 100644 --- a/sound/soc/tegra/Makefile +++ b/sound/soc/tegra/Makefile @@ -8,9 +8,11 @@ snd-soc-tegra20-i2s-objs := tegra20_i2s.o snd-soc-tegra20-spdif-objs := tegra20_spdif.o snd-soc-tegra30-ahub-objs := tegra30_ahub.o snd-soc-tegra30-i2s-objs := tegra30_i2s.o +snd-soc-tegra-cif-objs := tegra_cif.o obj-$(CONFIG_SND_SOC_TEGRA) += snd-soc-tegra-pcm.o obj-$(CONFIG_SND_SOC_TEGRA) += snd-soc-tegra-utils.o +obj-$(CONFIG_SND_SOC_TEGRA) += snd-soc-tegra-cif.o obj-$(CONFIG_SND_SOC_TEGRA20_AC97) += snd-soc-tegra20-ac97.o obj-$(CONFIG_SND_SOC_TEGRA20_DAS) += snd-soc-tegra20-das.o obj-$(CONFIG_SND_SOC_TEGRA20_I2S) += snd-soc-tegra20-i2s.o diff --git a/sound/soc/tegra/tegra_cif.c b/sound/soc/tegra/tegra_cif.c new file mode 100644 index 0000000..242ae34 --- /dev/null +++ b/sound/soc/tegra/tegra_cif.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * tegra_cif.c - Tegra Audio CIF Programming for AHUB modules + * + * Copyright (c) 2020 NVIDIA CORPORATION. All rights reserved. + * + */ + +#include <linux/module.h> +#include <linux/regmap.h> +#include "tegra_cif.h" + +void tegra_set_cif(struct regmap *regmap, unsigned int reg, + struct tegra_cif_conf *conf) +{ + unsigned int value; + + value = (conf->threshold << TEGRA_ACIF_CTRL_FIFO_TH_SHIFT) | + ((conf->audio_ch - 1) << TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT) | + ((conf->client_ch - 1) << TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT) | + (conf->audio_bits << TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT) | + (conf->client_bits << TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT) | + (conf->expand << TEGRA_ACIF_CTRL_EXPAND_SHIFT) | + (conf->stereo_conv << TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT) | + (conf->replicate << TEGRA_ACIF_CTRL_REPLICATE_SHIFT) | + (conf->truncate << TEGRA_ACIF_CTRL_TRUNCATE_SHIFT) | + (conf->mono_conv << TEGRA_ACIF_CTRL_MONO_CONV_SHIFT); + + regmap_update_bits(regmap, reg, TEGRA_ACIF_UPDATE_MASK, value); +} +EXPORT_SYMBOL_GPL(tegra_set_cif); + +MODULE_DESCRIPTION("Tegra Audio Client Interface (ACIF) driver"); +MODULE_LICENSE("GPL v2"); diff --git a/sound/soc/tegra/tegra_cif.h b/sound/soc/tegra/tegra_cif.h new file mode 100644 index 0000000..fb55812 --- /dev/null +++ b/sound/soc/tegra/tegra_cif.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * tegra_cif.h - TEGRA Audio CIF Programming + * + * Copyright (c) 2020 NVIDIA CORPORATION. All rights reserved. + * + */ + +#ifndef __TEGRA_CIF_H__ +#define __TEGRA_CIF_H__ + +#define TEGRA_ACIF_CTRL_FIFO_TH_SHIFT 24 +#define TEGRA_ACIF_CTRL_AUDIO_CH_SHIFT 20 +#define TEGRA_ACIF_CTRL_CLIENT_CH_SHIFT 16 +#define TEGRA_ACIF_CTRL_AUDIO_BITS_SHIFT 12 +#define TEGRA_ACIF_CTRL_CLIENT_BITS_SHIFT 8 +#define TEGRA_ACIF_CTRL_EXPAND_SHIFT 6 +#define TEGRA_ACIF_CTRL_STEREO_CONV_SHIFT 4 +#define TEGRA_ACIF_CTRL_REPLICATE_SHIFT 3 +#define TEGRA_ACIF_CTRL_TRUNCATE_SHIFT 1 +#define TEGRA_ACIF_CTRL_MONO_CONV_SHIFT 0 + +/* AUDIO/CLIENT_BITS values */ +#define TEGRA_ACIF_BITS_8 1 +#define TEGRA_ACIF_BITS_16 3 +#define TEGRA_ACIF_BITS_24 5 +#define TEGRA_ACIF_BITS_32 7 + +#define TEGRA_ACIF_UPDATE_MASK 0x3ffffffb + +struct tegra_cif_conf { + unsigned int threshold; + unsigned int audio_ch; + unsigned int client_ch; + unsigned int audio_bits; + unsigned int client_bits; + unsigned int expand; + unsigned int stereo_conv; + unsigned int replicate; + unsigned int truncate; + unsigned int mono_conv; +}; + +void tegra_set_cif(struct regmap *regmap, unsigned int reg, + struct tegra_cif_conf *conf); + +#endif
Audio Client Interface (CIF) is a proprietary interface employed to route audio samples through Audio Hub (AHUB) components by inter connecting the various modules. This patch exports a helper function tegra_set_cif() which can be used, for now, to program CIF on Tegra210 and later Tegra generations. Later it can be extended to include helpers for legacy chips as well. Signed-off-by: Sameer Pujar <spujar@nvidia.com> --- sound/soc/tegra/Makefile | 2 ++ sound/soc/tegra/tegra_cif.c | 34 ++++++++++++++++++++++++++++++++ sound/soc/tegra/tegra_cif.h | 47 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 sound/soc/tegra/tegra_cif.c create mode 100644 sound/soc/tegra/tegra_cif.h