diff mbox

[16/31] ASoC: tegra: use reset framework

Message ID 1384548866-13141-17-git-send-email-swarren@wwwdotorg.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Warren Nov. 15, 2013, 8:54 p.m. UTC 
From: Stephen Warren <swarren@nvidia.com>

Tegra's clock driver now provides an implementation of the common
reset API (include/linux/reset.h). Use this instead of the old Tegra-
specific API; that will soon be removed.

This change also renames "clock"/"clk" to "modules"/"mod" in symbols
related to entries in configlink_clocks[], since:
- We don't care about clock handles any more, but rather reset handles,
  so the old name isn't applicable.
- It really is a list of modules on the bus, about which we currently
  only care about reset handles.
If we start caring about any other aspect of the modules in the future,
we won't have to rename all these symbols again.

Cc: treding@nvidia.com
Cc: pdeschrijver@nvidia.com
Cc: linux-tegra@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: alsa-devel@alsa-project.org
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This patch is part of a series with strong internal depdendencies. I'm
looking for an ack so that I can take the entire series through the Tegra
and arm-soc trees. The series will be part of a stable branch that can be
merged into other subsystems if needed to avoid/resolve dependencies.
---
 sound/soc/tegra/Kconfig        |  2 ++
 sound/soc/tegra/tegra30_ahub.c | 70 +++++++++++++++++++++++-------------------
 sound/soc/tegra/tegra30_ahub.h |  2 +-
 3 files changed, 41 insertions(+), 33 deletions(-)

Comments

Mark Brown Nov. 16, 2013, 9:55 a.m. UTC | #1 
On Fri, Nov 15, 2013 at 01:54:11PM -0700, Stephen Warren wrote:

> @@ -1,6 +1,8 @@
>  config SND_SOC_TEGRA
>  	tristate "SoC Audio for the Tegra System-on-Chip"
>  	depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST
> +	depends on COMMON_CLK
> +	depends on RESET_CONTROLLER

Do you depend on COMMON_CLK here?  I only noticed reset controller API
dependencies here but perhaps I missed this (or it's fixing a dependency
that should be there already).
Stephen Warren Nov. 18, 2013, 5:21 p.m. UTC | #2 
On 11/16/2013 02:55 AM, Mark Brown wrote:
> On Fri, Nov 15, 2013 at 01:54:11PM -0700, Stephen Warren wrote:
> 
>> @@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the
>> Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA)
>> || COMPILE_TEST +	depends on COMMON_CLK +	depends on
>> RESET_CONTROLLER
> 
> Do you depend on COMMON_CLK here?  I only noticed reset controller
> API dependencies here but perhaps I missed this (or it's fixing a
> dependency that should be there already).

It's fixing a dependency that should already be there, in the
COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case,
COMMON_CLOCK is always selected.

Do you want me to split this out into a separate patch? If so, I'd
prefer not to apply that separate patch immediately to 3.13 as a fix,
since then it'd delay applying this series until after -rc2 is out,
unless you can get the fix into -rc1 quickly...
Mark Brown Nov. 18, 2013, 6:37 p.m. UTC | #3 
On Mon, Nov 18, 2013 at 10:21:16AM -0700, Stephen Warren wrote:

> It's fixing a dependency that should already be there, in the
> COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case,
> COMMON_CLOCK is always selected.

> Do you want me to split this out into a separate patch? If so, I'd
> prefer not to apply that separate patch immediately to 3.13 as a fix,
> since then it'd delay applying this series until after -rc2 is out,
> unless you can get the fix into -rc1 quickly...

I don't really care, it was just that I was looking for something to do
with clocks in the patch and couldn't find anything.  Perhaps a note in
the changelog if you need to respin so I don't forget and say the same
thing again.
Stephen Warren Nov. 25, 2013, 9:56 p.m. UTC | #4 
On 11/18/2013 11:37 AM, Mark Brown wrote:
> On Mon, Nov 18, 2013 at 10:21:16AM -0700, Stephen Warren wrote:
> 
>> It's fixing a dependency that should already be there, in the 
>> COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, 
>> COMMON_CLOCK is always selected.
> 
>> Do you want me to split this out into a separate patch? If so,
>> I'd prefer not to apply that separate patch immediately to 3.13
>> as a fix, since then it'd delay applying this series until after
>> -rc2 is out, unless you can get the fix into -rc1 quickly...
> 
> I don't really care, it was just that I was looking for something
> to do with clocks in the patch and couldn't find anything.  Perhaps
> a note in the changelog if you need to respin so I don't forget and
> say the same thing again.

I added the following to the changelog:

----------
Note: The addition of "depends COMMON_CLOCK" is something that was
missing before, not a new requirement.
----------

You didn't ack this one patch; I assume that was just an oversight?
Mark Brown Nov. 26, 2013, 1:14 p.m. UTC | #5 
On Mon, Nov 25, 2013 at 02:56:23PM -0700, Stephen Warren wrote:

> You didn't ack this one patch; I assume that was just an oversight?

No, it was because it looked incorrect based on the lack of tie in
between the description and the code.
Stephen Warren Nov. 26, 2013, 4:31 p.m. UTC | #6 
On 11/26/2013 06:14 AM, Mark Brown wrote:
> On Mon, Nov 25, 2013 at 02:56:23PM -0700, Stephen Warren wrote:
> 
>> You didn't ack this one patch; I assume that was just an
>> oversight?
> 
> No, it was because it looked incorrect based on the lack of tie in 
> between the description and the code.

Hmm. You had asked:

>> @@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the
>> Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA)
>> || COMPILE_TEST +	depends on COMMON_CLK +	depends on
>> RESET_CONTROLLER
> 
> Do you depend on COMMON_CLK here?  I only noticed reset controller
> API dependencies here but perhaps I missed this (or it's fixing a
> dependency that should be there already).

I responded:

> It's fixing a dependency that should already be there, in the 
> COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, 
> COMMON_CLOCK is always selected.
> 
> Do you want me to split this out into a separate patch? If so, I'd 
> prefer not to apply that separate patch immediately to 3.13 as a
> fix, since then it'd delay applying this series until after -rc2 is
> out, unless you can get the fix into -rc1 quickly...

(although at this point in time, the DMA patches which this depend on
aren't likely to be ready soon enough for the delay to matter, so I
could send the addition of depends COMMON_CLK as a separate patch for
-rc2 if you want)

and you said:

> I don't really care, it was just that I was looking for something
> to do with clocks in the patch and couldn't find anything.  Perhaps
> a note in the changelog if you need to respin so I don't forget and
> say the same thing again.

... so, I thought you were OK with that one issue. Were there other
issues you didn't mention before?
Mark Brown Nov. 26, 2013, 6:37 p.m. UTC | #7 
On Tue, Nov 26, 2013 at 09:31:25AM -0700, Stephen Warren wrote:

> ... so, I thought you were OK with that one issue. Were there other
> issues you didn't mention before?

Probably not but I'd need to reread it, I don't think I got that much
further than noticing that there weren't any clock changes (the fact
that the clock dependency was getting added in a DMA series set off
alarm bells).  To be honest given the number of revisions that seem to
be required I'd been expecting to see a respin of the series, the ASoC
generic DMA changes did need a respin before they can be merged and I do
want to get them into the ASoC tree.
Stephen Warren Nov. 26, 2013, 6:45 p.m. UTC | #8 
On 11/26/2013 11:37 AM, Mark Brown wrote:
> On Tue, Nov 26, 2013 at 09:31:25AM -0700, Stephen Warren wrote:
> 
>> ... so, I thought you were OK with that one issue. Were there
>> other issues you didn't mention before?
> 
> Probably not but I'd need to reread it, I don't think I got that
> much further than noticing that there weren't any clock changes
> (the fact that the clock dependency was getting added in a DMA
> series set off alarm bells).  To be honest given the number of
> revisions that seem to be required I'd been expecting to see a
> respin of the series, the ASoC generic DMA changes did need a
> respin before they can be merged and I do want to get them into the
> ASoC tree.

OK, I'll repost.

I'm waiting to repost the ASoC core changes until the dmaengine API
change settles. Hopefully very soon.
diff mbox

Patch

diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
index 8fc653ca3ab4..896292bb853f 100644
--- a/sound/soc/tegra/Kconfig
+++ b/sound/soc/tegra/Kconfig
@@ -1,6 +1,8 @@ 
 config SND_SOC_TEGRA
 	tristate "SoC Audio for the Tegra System-on-Chip"
 	depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST
+	depends on COMMON_CLK
+	depends on RESET_CONTROLLER
 	select REGMAP_MMIO
 	select SND_SOC_GENERIC_DMAENGINE_PCM
 	help
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c
index 31154338c1eb..5ce00dc48c44 100644
--- a/sound/soc/tegra/tegra30_ahub.c
+++ b/sound/soc/tegra/tegra30_ahub.c
@@ -24,6 +24,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/clk/tegra.h>
 #include <sound/soc.h>
@@ -301,27 +302,27 @@  int tegra30_ahub_unset_rx_cif_source(enum tegra30_ahub_rxcif rxcif)
 }
 EXPORT_SYMBOL_GPL(tegra30_ahub_unset_rx_cif_source);
 
-#define CLK_LIST_MASK_TEGRA30	BIT(0)
-#define CLK_LIST_MASK_TEGRA114	BIT(1)
+#define MOD_LIST_MASK_TEGRA30	BIT(0)
+#define MOD_LIST_MASK_TEGRA114	BIT(1)
 
-#define CLK_LIST_MASK_TEGRA30_OR_LATER \
-		(CLK_LIST_MASK_TEGRA30 | CLK_LIST_MASK_TEGRA114)
+#define MOD_LIST_MASK_TEGRA30_OR_LATER \
+		(MOD_LIST_MASK_TEGRA30 | MOD_LIST_MASK_TEGRA114)
 
 static const struct {
-	const char *clk_name;
-	u32 clk_list_mask;
-} configlink_clocks[] = {
-	{ "i2s0", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "i2s1", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "i2s2", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "i2s3", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "i2s4", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "dam0", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "dam1", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "dam2", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "spdif_in", CLK_LIST_MASK_TEGRA30_OR_LATER },
-	{ "amx", CLK_LIST_MASK_TEGRA114 },
-	{ "adx", CLK_LIST_MASK_TEGRA114 },
+	const char *rst_name;
+	u32 mod_list_mask;
+} configlink_mods[] = {
+	{ "i2s0", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "i2s1", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "i2s2", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "i2s3", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "i2s4", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "dam0", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "dam1", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "dam2", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "spdif", MOD_LIST_MASK_TEGRA30_OR_LATER },
+	{ "amx", MOD_LIST_MASK_TEGRA114 },
+	{ "adx", MOD_LIST_MASK_TEGRA114 },
 };
 
 #define LAST_REG(name) \
@@ -450,17 +451,17 @@  static const struct regmap_config tegra30_ahub_ahub_regmap_config = {
 };
 
 static struct tegra30_ahub_soc_data soc_data_tegra30 = {
-	.clk_list_mask = CLK_LIST_MASK_TEGRA30,
+	.mod_list_mask = MOD_LIST_MASK_TEGRA30,
 	.set_audio_cif = tegra30_ahub_set_cif,
 };
 
 static struct tegra30_ahub_soc_data soc_data_tegra114 = {
-	.clk_list_mask = CLK_LIST_MASK_TEGRA114,
+	.mod_list_mask = MOD_LIST_MASK_TEGRA114,
 	.set_audio_cif = tegra30_ahub_set_cif,
 };
 
 static struct tegra30_ahub_soc_data soc_data_tegra124 = {
-	.clk_list_mask = CLK_LIST_MASK_TEGRA114,
+	.mod_list_mask = MOD_LIST_MASK_TEGRA114,
 	.set_audio_cif = tegra124_ahub_set_cif,
 };
 
@@ -475,7 +476,7 @@  static int tegra30_ahub_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *match;
 	const struct tegra30_ahub_soc_data *soc_data;
-	struct clk *clk;
+	struct reset_control *rst;
 	int i;
 	struct resource *res0, *res1, *region;
 	u32 of_dma[2];
@@ -495,19 +496,24 @@  static int tegra30_ahub_probe(struct platform_device *pdev)
 	 * operate correctly, all devices on this bus must be out of reset.
 	 * Ensure that here.
 	 */
-	for (i = 0; i < ARRAY_SIZE(configlink_clocks); i++) {
-		if (!(configlink_clocks[i].clk_list_mask &
-					soc_data->clk_list_mask))
+	for (i = 0; i < ARRAY_SIZE(configlink_mods); i++) {
+		if (!(configlink_mods[i].mod_list_mask &
+					soc_data->mod_list_mask))
 			continue;
-		clk = clk_get(&pdev->dev, configlink_clocks[i].clk_name);
-		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "Can't get clock %s\n",
-				configlink_clocks[i].clk_name);
-			ret = PTR_ERR(clk);
+
+		rst = reset_control_get(&pdev->dev,
+					configlink_mods[i].rst_name);
+		if (IS_ERR(rst)) {
+			dev_err(&pdev->dev, "Can't get reset %s\n",
+				configlink_mods[i].rst_name);
+			ret = PTR_ERR(rst);
 			goto err;
 		}
-		tegra_periph_reset_deassert(clk);
-		clk_put(clk);
+
+		ret = reset_control_deassert(rst);
+		reset_control_put(rst);
+		if (ret)
+			goto err;
 	}
 
 	ahub = devm_kzalloc(&pdev->dev, sizeof(struct tegra30_ahub),
diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h
index d67321d90faa..1383f8cd3572 100644
--- a/sound/soc/tegra/tegra30_ahub.h
+++ b/sound/soc/tegra/tegra30_ahub.h
@@ -502,7 +502,7 @@  void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg,
 			   struct tegra30_ahub_cif_conf *conf);
 
 struct tegra30_ahub_soc_data {
-	u32 clk_list_mask;
+	u32 mod_list_mask;
 	void (*set_audio_cif)(struct regmap *regmap,
 			      unsigned int reg,
 			      struct tegra30_ahub_cif_conf *conf);