diff mbox series

[v2,2/9] ASoC: tegra: add support for CIF programming

Message ID 1580380422-3431-3-git-send-email-spujar@nvidia.com (mailing list archive)
State New, archived
Headers show
Series add ASoC components for AHUB | expand

Commit Message

Sameer Pujar Jan. 30, 2020, 10:33 a.m. UTC 
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

Comments

Dmitry Osipenko Feb. 5, 2020, 5:02 p.m. UTC | #1 
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).
Sameer Pujar Feb. 6, 2020, 11:56 a.m. UTC | #2 
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).
Dmitry Osipenko Feb. 6, 2020, 4:49 p.m. UTC | #3 
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.
Sameer Pujar Feb. 7, 2020, 11:03 a.m. UTC | #4 
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 mbox series

Patch

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