[v3] ASoC: AMD: Enable/Disable auxiliary clock via common clock framework
diff mbox

Message ID 1521605103-13574-1-git-send-email-akshu.agrawal@amd.com
State New
Headers show

Commit Message

Agrawal, Akshu March 21, 2018, 4:05 a.m. UTC
This enables/disables and sets auxiliary clock at 25Mhz. It uses
common clock framework for proper ref counting. This approach will
save power in comparison to keeping it always On in firmware.

V2: Correcting the pin to OSCOUT1 from OSCOUT2
V3: Fix error/warnings from kbuild test

TEST= aplay -vv <file>
check register to see clock enabled
kill aplay
check register to see clock disabled

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
 sound/soc/amd/acp-da7219-max98357a.c | 135 ++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 2 deletions(-)

Comments

kernel test robot March 23, 2018, 3:45 p.m. UTC | #1
Hi Akshu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20180323]
[cannot apply to v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Akshu-Agrawal/ASoC-AMD-Enable-Disable-auxiliary-clock-via-common-clock-framework/20180323-220927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   sound/soc/amd/acp-da7219-max98357a.c:61:12: sparse: symbol 'da7219_dai_clk' was not declared. Should it be static?
>> sound/soc/amd/acp-da7219-max98357a.c:312:22: sparse: symbol 'acpd7219_clks_ops' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot March 23, 2018, 5:01 p.m. UTC | #2
Hi Akshu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on next-20180323]
[cannot apply to v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Akshu-Agrawal/ASoC-AMD-Enable-Disable-auxiliary-clock-via-common-clock-framework/20180323-220927
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All errors (new ones prefixed by >>):

   sound/soc/amd/acp-da7219-max98357a.c:54:16: error: field 'acp_clks_hw' has incomplete type
     struct clk_hw acp_clks_hw;
                   ^~~~~~~~~~~
   In file included from include/linux/ioport.h:13:0,
                    from include/linux/device.h:15,
                    from include/sound/core.h:25,
                    from sound/soc/amd/acp-da7219-max98357a.c:26:
   sound/soc/amd/acp-da7219-max98357a.c: In function 'acpd7219_clks_prepare':
   include/linux/kernel.h:931:32: error: dereferencing pointer to incomplete type 'struct clk_hw'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                                   ^~~~~~
   include/linux/compiler.h:316:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^~~~~~~~~
   include/linux/compiler.h:339:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:931:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:931:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:275:3: note: in expansion of macro 'container_of'
      container_of(hw, struct cz_clock, acp_clks_hw);
      ^~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c: At top level:
   sound/soc/amd/acp-da7219-max98357a.c:312:14: error: variable 'acpd7219_clks_ops' has initializer but incomplete type
    const struct clk_ops acpd7219_clks_ops = {
                 ^~~~~~~
>> sound/soc/amd/acp-da7219-max98357a.c:313:2: error: unknown field 'prepare' specified in initializer
     .prepare = acpd7219_clks_prepare,
     ^
   sound/soc/amd/acp-da7219-max98357a.c:313:13: warning: excess elements in struct initializer
     .prepare = acpd7219_clks_prepare,
                ^~~~~~~~~~~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:313:13: note: (near initialization for 'acpd7219_clks_ops')
>> sound/soc/amd/acp-da7219-max98357a.c:314:2: error: unknown field 'unprepare' specified in initializer
     .unprepare = acpd7219_clks_unprepare,
     ^
   sound/soc/amd/acp-da7219-max98357a.c:314:15: warning: excess elements in struct initializer
     .unprepare = acpd7219_clks_unprepare,
                  ^~~~~~~~~~~~~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:314:15: note: (near initialization for 'acpd7219_clks_ops')
>> sound/soc/amd/acp-da7219-max98357a.c:315:2: error: unknown field 'is_enabled' specified in initializer
     .is_enabled = acpd7219_clks_is_enabled,
     ^
   sound/soc/amd/acp-da7219-max98357a.c:315:16: warning: excess elements in struct initializer
     .is_enabled = acpd7219_clks_is_enabled,
                   ^~~~~~~~~~~~~~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:315:16: note: (near initialization for 'acpd7219_clks_ops')
   sound/soc/amd/acp-da7219-max98357a.c: In function 'register_acpd7219_clocks':
   sound/soc/amd/acp-da7219-max98357a.c:320:9: error: variable 'init' has initializer but incomplete type
     struct clk_init_data init = {};
            ^~~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:320:23: error: storage size of 'init' isn't known
     struct clk_init_data init = {};
                          ^~~~
>> sound/soc/amd/acp-da7219-max98357a.c:339:13: error: implicit declaration of function 'devm_clk_register' [-Werror=implicit-function-declaration]
     acp_clks = devm_clk_register(&dev, &cz_clock_obj->acp_clks_hw);
                ^~~~~~~~~~~~~~~~~
   sound/soc/amd/acp-da7219-max98357a.c:320:23: warning: unused variable 'init' [-Wunused-variable]
     struct clk_init_data init = {};
                          ^~~~
   sound/soc/amd/acp-da7219-max98357a.c: At top level:
   sound/soc/amd/acp-da7219-max98357a.c:312:22: error: storage size of 'acpd7219_clks_ops' isn't known
    const struct clk_ops acpd7219_clks_ops = {
                         ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/prepare +313 sound/soc/amd/acp-da7219-max98357a.c

   270	
   271	static int acpd7219_clks_prepare(struct clk_hw *hw)
   272	{
   273		u32 reg_val;
   274		struct cz_clock *cz_clock_obj =
 > 275			container_of(hw, struct cz_clock, acp_clks_hw);
   276		void __iomem *base = cz_clock_obj->res_base;
   277	
   278		reg_val = readl(base + MISCCLKCNTL1);
   279		reg_val &= ~(0x1 << OSCCLKENB);
   280		writel(reg_val, base + MISCCLKCNTL1);
   281		reg_val = readl(base + CLKDRVSTR2);
   282		reg_val |= (0x1 << OSCOUT1CLK25MHZ);
   283		writel(reg_val, base + CLKDRVSTR2);
   284	
   285		return 0;
   286	}
   287	
   288	static void acpd7219_clks_unprepare(struct clk_hw *hw)
   289	{
   290		u32 reg_val;
   291		struct cz_clock *cz_clock_obj =
   292			container_of(hw, struct cz_clock, acp_clks_hw);
   293		void __iomem *base = cz_clock_obj->res_base;
   294	
   295		reg_val = readl(base + MISCCLKCNTL1);
   296		reg_val |= (0x1 << OSCCLKENB);
   297		writel(reg_val, base + MISCCLKCNTL1);
   298	}
   299	
   300	static int acpd7219_clks_is_enabled(struct clk_hw *hw)
   301	{
   302		u32 reg_val;
   303		struct cz_clock *cz_clock_obj =
   304			container_of(hw, struct cz_clock, acp_clks_hw);
   305		void __iomem *base = cz_clock_obj->res_base;
   306	
   307		reg_val = readl(base + MISCCLKCNTL1);
   308	
   309		return !(reg_val & OSCCLKENB);
   310	}
   311	
   312	const struct clk_ops acpd7219_clks_ops = {
 > 313		.prepare = acpd7219_clks_prepare,
 > 314		.unprepare = acpd7219_clks_unprepare,
 > 315		.is_enabled = acpd7219_clks_is_enabled,
   316	};
   317	
   318	static int register_acpd7219_clocks(struct platform_device *pdev)
   319	{
   320		struct clk_init_data init = {};
   321		struct clk *acp_clks;
   322		struct clk_lookup *acp_clks_lookup;
   323		struct cz_clock *cz_clock_obj;
   324		struct resource *res;
   325		struct device dev = pdev->dev;
   326	
   327		cz_clock_obj = kzalloc(sizeof(struct cz_clock), GFP_KERNEL);
   328		if (!cz_clock_obj)
   329			return -ENOMEM;
   330	
   331		cz_clock_obj->acp_clks_name = "acpd7219-clks";
   332	
   333		init.parent_names = NULL;
   334		init.num_parents = 0;
   335		init.name = cz_clock_obj->acp_clks_name;
   336		init.ops = &acpd7219_clks_ops;
   337		cz_clock_obj->acp_clks_hw.init = &init;
   338	
 > 339		acp_clks = devm_clk_register(&dev, &cz_clock_obj->acp_clks_hw);
   340		if (IS_ERR(acp_clks))
   341		{
   342			dev_err(&dev, "Failed to register DAI clocks: %ld\n",
   343				 PTR_ERR(acp_clks));
   344			return -EINVAL;
   345		}
   346		cz_clock_obj->acp_clks = acp_clks;
   347	
   348		acp_clks_lookup = clkdev_create(acp_clks, cz_clock_obj->acp_clks_name,
   349						"%s", dev_name(&dev));
   350		if (!acp_clks_lookup)
   351			dev_warn(&dev, "Failed to create DAI clkdev");
   352		else
   353			cz_clock_obj->acp_clks_lookup = acp_clks_lookup;
   354	
   355		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   356		if (!res) {
   357			dev_err(&pdev->dev, "Failed to get misc io resource.\n");
   358			return -EINVAL;
   359		}
   360		cz_clock_obj->res_base = devm_ioremap_nocache(&pdev->dev, res->start,
   361						resource_size(res));
   362		if (!cz_clock_obj->res_base)
   363			return -ENOMEM;
   364	
   365		return 0;
   366	}
   367	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch
diff mbox

diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c
index 99c6b5c..baea370 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -30,10 +30,13 @@ 
 #include <sound/soc-dapm.h>
 #include <sound/jack.h>
 #include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
 #include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/acpi.h>
+#include <linux/types.h>
 
 #include "../codecs/da7219.h"
 #include "../codecs/da7219-aad.h"
@@ -41,6 +44,18 @@ 
 #define CZ_PLAT_CLK 24000000
 #define MCLK_RATE 24576000
 #define DUAL_CHANNEL		2
+#define CLKDRVSTR2	0x28
+#define MISCCLKCNTL1	0x40
+#define OSCCLKENB	2
+#define OSCOUT1CLK25MHZ	16
+
+struct cz_clock {
+	const char* acp_clks_name;
+	struct clk_hw acp_clks_hw;
+	struct clk_lookup *acp_clks_lookup;
+	struct clk *acp_clks;
+	void __iomem *res_base;
+};
 
 static struct snd_soc_jack cz_jack;
 struct clk *da7219_dai_clk;
@@ -91,6 +106,8 @@  static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
 {
 	int ret = 0;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct clk *acpd7219_clk;
 
 	ret = clk_prepare_enable(da7219_dai_clk);
 	if (ret < 0) {
@@ -98,13 +115,27 @@  static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
+	acpd7219_clk = clk_get(card->dev, "acpd7219-clks");
+	ret = clk_prepare_enable(acpd7219_clk);
+	if (ret < 0) {
+		dev_err(rtd->dev, "can't enable oscillator clock %d\n", ret);
+		return ret;
+	}
+
 	return ret;
 }
 
 static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
 {
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct clk *acpd7219_clk;
+
 	clk_disable_unprepare(da7219_dai_clk);
 
+	acpd7219_clk = clk_get(card->dev, "acpd7219-clks");
+	clk_disable_unprepare(acpd7219_clk);
+
 	return 0;
 }
 
@@ -237,9 +268,106 @@  static int cz_fe_startup(struct snd_pcm_substream *substream)
 	.num_controls = ARRAY_SIZE(cz_mc_controls),
 };
 
+static int acpd7219_clks_prepare(struct clk_hw *hw)
+{
+	u32 reg_val;
+	struct cz_clock *cz_clock_obj =
+		container_of(hw, struct cz_clock, acp_clks_hw);
+	void __iomem *base = cz_clock_obj->res_base;
+
+	reg_val = readl(base + MISCCLKCNTL1);
+	reg_val &= ~(0x1 << OSCCLKENB);
+	writel(reg_val, base + MISCCLKCNTL1);
+	reg_val = readl(base + CLKDRVSTR2);
+	reg_val |= (0x1 << OSCOUT1CLK25MHZ);
+	writel(reg_val, base + CLKDRVSTR2);
+
+	return 0;
+}
+
+static void acpd7219_clks_unprepare(struct clk_hw *hw)
+{
+	u32 reg_val;
+	struct cz_clock *cz_clock_obj =
+		container_of(hw, struct cz_clock, acp_clks_hw);
+	void __iomem *base = cz_clock_obj->res_base;
+
+	reg_val = readl(base + MISCCLKCNTL1);
+	reg_val |= (0x1 << OSCCLKENB);
+	writel(reg_val, base + MISCCLKCNTL1);
+}
+
+static int acpd7219_clks_is_enabled(struct clk_hw *hw)
+{
+	u32 reg_val;
+	struct cz_clock *cz_clock_obj =
+		container_of(hw, struct cz_clock, acp_clks_hw);
+	void __iomem *base = cz_clock_obj->res_base;
+
+	reg_val = readl(base + MISCCLKCNTL1);
+
+	return !(reg_val & OSCCLKENB);
+}
+
+const struct clk_ops acpd7219_clks_ops = {
+	.prepare = acpd7219_clks_prepare,
+	.unprepare = acpd7219_clks_unprepare,
+	.is_enabled = acpd7219_clks_is_enabled,
+};
+
+static int register_acpd7219_clocks(struct platform_device *pdev)
+{
+	struct clk_init_data init = {};
+	struct clk *acp_clks;
+	struct clk_lookup *acp_clks_lookup;
+	struct cz_clock *cz_clock_obj;
+	struct resource *res;
+	struct device dev = pdev->dev;
+
+	cz_clock_obj = kzalloc(sizeof(struct cz_clock), GFP_KERNEL);
+	if (!cz_clock_obj)
+		return -ENOMEM;
+
+	cz_clock_obj->acp_clks_name = "acpd7219-clks";
+
+	init.parent_names = NULL;
+	init.num_parents = 0;
+	init.name = cz_clock_obj->acp_clks_name;
+	init.ops = &acpd7219_clks_ops;
+	cz_clock_obj->acp_clks_hw.init = &init;
+
+	acp_clks = devm_clk_register(&dev, &cz_clock_obj->acp_clks_hw);
+	if (IS_ERR(acp_clks))
+	{
+		dev_err(&dev, "Failed to register DAI clocks: %ld\n",
+			 PTR_ERR(acp_clks));
+		return -EINVAL;
+	}
+	cz_clock_obj->acp_clks = acp_clks;
+
+	acp_clks_lookup = clkdev_create(acp_clks, cz_clock_obj->acp_clks_name,
+					"%s", dev_name(&dev));
+	if (!acp_clks_lookup)
+		dev_warn(&dev, "Failed to create DAI clkdev");
+	else
+		cz_clock_obj->acp_clks_lookup = acp_clks_lookup;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Failed to get misc io resource.\n");
+		return -EINVAL;
+	}
+	cz_clock_obj->res_base = devm_ioremap_nocache(&pdev->dev, res->start,
+					resource_size(res));
+	if (!cz_clock_obj->res_base)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int cz_probe(struct platform_device *pdev)
 {
-	int ret;
+	int ret = 0;
 	struct snd_soc_card *card;
 
 	card = &cz_card;
@@ -252,7 +380,10 @@  static int cz_probe(struct platform_device *pdev)
 				cz_card.name, ret);
 		return ret;
 	}
-	return 0;
+
+	ret = register_acpd7219_clocks(pdev);
+
+	return ret;
 }
 
 static const struct acpi_device_id cz_audio_acpi_match[] = {