| Message ID | 5360E5E7.8090807@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 30, 2014 at 05:30:39PM +0530, Tushar Behera wrote: > XCLKOUT mux register (0x10040a00) is not part of core clock SFR range, > rather it is part of pmu-system-controller node. > One option would be to add a clock provider for XCLKOUT. That would > require me to extend current clock driver to get the > pmu-system-controller phandle which in turn is dependent on the patch to > early initialize syscon driver.[1] > Will it be okay to configure XCLKOUT in snow sound-card driver by taking > the phandle of pmu-system-controller from device tree? The > implementation is provided below. I can't think of any reason why we'd not want to have this visible as a clock? Ideally we don't want to have any ASoC specific clock APIs when we're done, the reason we do at the minute is that the clock API isn't available as standard.
On 04/30/2014 11:33 PM, Mark Brown wrote: > On Wed, Apr 30, 2014 at 05:30:39PM +0530, Tushar Behera wrote: > >> XCLKOUT mux register (0x10040a00) is not part of core clock SFR range, >> rather it is part of pmu-system-controller node. > >> One option would be to add a clock provider for XCLKOUT. That would >> require me to extend current clock driver to get the >> pmu-system-controller phandle which in turn is dependent on the patch to >> early initialize syscon driver.[1] > >> Will it be okay to configure XCLKOUT in snow sound-card driver by taking >> the phandle of pmu-system-controller from device tree? The >> implementation is provided below. > > I can't think of any reason why we'd not want to have this visible as a > clock? Ideally we don't want to have any ASoC specific clock APIs when > we're done, the reason we do at the minute is that the clock API isn't > available as standard. > Okay, I will extend the existing clock driver to support XCLKOUT. Of the many parents of XCLKOUT, we need to set XXTI clock as the parent. Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent" (XXTI) to sound-card driver via DT and do the necessary reparenting during the sound-card driver probe call? Else, we can push that change to bootloader (to set the XCLKOUT mux register) and only enable/disable the clock in sound-card driver. Let me know your opinion on this.
On Thu, May 01, 2014 at 04:59:08PM +0530, Tushar Behera wrote: > Okay, I will extend the existing clock driver to support XCLKOUT. It may make more sense to add another clock driver for this clock depending on how things are done, I don't know. > Of the many parents of XCLKOUT, we need to set XXTI clock as the parent. > Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent" > (XXTI) to sound-card driver via DT and do the necessary reparenting > during the sound-card driver probe call? No, that's not OK at all, it won't allow for configuration of the system. This is what I was talking about when I was talking the clock framework extensions to allow the clock tree to be configured using DT, that would allow the settings to be put in DT. > Else, we can push that change to bootloader (to set the XCLKOUT mux > register) and only enable/disable the clock in sound-card driver. That's not going to work given that the existing bootloaders don't do this.
Hi, On Thu, May 1, 2014 at 7:15 AM, Mark Brown <broonie@kernel.org> wrote: > On Thu, May 01, 2014 at 04:59:08PM +0530, Tushar Behera wrote: > >> Okay, I will extend the existing clock driver to support XCLKOUT. > > It may make more sense to add another clock driver for this clock > depending on how things are done, I don't know. > >> Of the many parents of XCLKOUT, we need to set XXTI clock as the parent. >> Is it okay if we pass two clocks "mclk" (XCLKOUT) and "mclk_parent" >> (XXTI) to sound-card driver via DT and do the necessary reparenting >> during the sound-card driver probe call? > > No, that's not OK at all, it won't allow for configuration of the > system. This is what I was talking about when I was talking the clock > framework extensions to allow the clock tree to be configured using DT, > that would allow the settings to be put in DT. > >> Else, we can push that change to bootloader (to set the XCLKOUT mux >> register) and only enable/disable the clock in sound-card driver. > > That's not going to work given that the existing bootloaders don't do > this. This is exactly the thing (expected clock parenting) we agreed could be put in the device tree I think. ...but I don't know that anyone proposed exactly how that would work. NOTE: in existing ChromeOS this type of thing is done a tiny amount in <https://chromium.googlesource.com/chromiumos/third_party/kernel-next/+/chromeos-3.8/arch/arm/mach-exynos/mach-exynos5-dt.c>. See apply_clock_muxing(). -Doug -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 01, 2014 at 08:54:14AM -0700, Doug Anderson wrote: > This is exactly the thing (expected clock parenting) we agreed could > be put in the device tree I think. ...but I don't know that anyone > proposed exactly how that would work. There's patches been posted by (IIRC) Sylvester Nawrocki for this which I think Mike was basically happy with - I don't immediately see them in -next though, but I may be looking in the wrong place.
On 05/01/2014 10:10 PM, Mark Brown wrote: > On Thu, May 01, 2014 at 08:54:14AM -0700, Doug Anderson wrote: > >> This is exactly the thing (expected clock parenting) we agreed could >> be put in the device tree I think. ...but I don't know that anyone >> proposed exactly how that would work. > > There's patches been posted by (IIRC) Sylvester Nawrocki for this which > I think Mike was basically happy with - I don't immediately see them in > -next though, but I may be looking in the wrong place. > Right, there are some patches from Sylwester regarding this. I dont' think the patches are accepted yet as Rob has some reservations against the clock parents. https://lkml.org/lkml/2014/4/9/173
On Fri, May 02, 2014 at 10:26:08AM +0530, Tushar Behera wrote: > On 05/01/2014 10:10 PM, Mark Brown wrote: > > There's patches been posted by (IIRC) Sylvester Nawrocki for this which > > I think Mike was basically happy with - I don't immediately see them in > > -next though, but I may be looking in the wrong place. > Right, there are some patches from Sylwester regarding this. I dont' > think the patches are accepted yet as Rob has some reservations against > the clock parents. > https://lkml.org/lkml/2014/4/9/173 Well, OK - that's the bit to fix. Speaking to Mike the other day he said there was also another competing implementation of this.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 05/02/2014 10:26 PM, Mark Brown wrote: > On Fri, May 02, 2014 at 10:26:08AM +0530, Tushar Behera wrote: >> On 05/01/2014 10:10 PM, Mark Brown wrote: > >>> There's patches been posted by (IIRC) Sylvester Nawrocki for >>> this which I think Mike was basically happy with - I don't >>> immediately see them in -next though, but I may be looking in >>> the wrong place. > >> Right, there are some patches from Sylwester regarding this. I >> dont' think the patches are accepted yet as Rob has some >> reservations against the clock parents. > >> https://lkml.org/lkml/2014/4/9/173 > > Well, OK - that's the bit to fix. Speaking to Mike the other day > he said there was also another competing implementation of this. > Instead of setting the parent clock, setting the desired rate on XCLKOUT works for audio playback. Will it be okay to do a 'clk_set_rate(mclk, 24000000)' in sound card driver or should we wait for Sylwester's patch to get merged? - -- Tushar Behera -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTefPOAAoJELqclMPPkq4N6UIH/i5W7KYjyKTQtX+AGdUmJwIx 3TTwyWVz5YQ6gFpLHwwy5AwIkj4mygox2e7l0qWU6yOXCMceaXrvzxOK+B/LRXlp +s8xjd+C+Idxhs0T6V4GkJRA9jtari3gcIGfz4tFtWY5O0l0eNTUduILhAvmE/Sw c4m8aOwK33P0kvkTIp/ShSujYa8v82XosMRuf4P0iEz9I3qw3syJkWWvqNZFrnPs B7i+9ZiZkyttsUVBOAWMBfOdLP5sMtxKDdk1GTkN1plxAE8lKdNVATR5htQL4bxo ZFcWJxE14pWQ770gEJVXkDT9o8dhmVNrDVp4eGe3m+43gN+Bssh+9kgdxb3vmIY= =gXaJ -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 19, 2014 at 05:36:38PM +0530, Tushar Behera wrote: > Instead of setting the parent clock, setting the desired rate on > XCLKOUT works for audio playback. > Will it be okay to do a 'clk_set_rate(mclk, 24000000)' in sound card > driver or should we wait for Sylwester's patch to get merged? Yes, that's fine - going further back into the clock tree isn't.
diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c index 0fa89a4..d7fe7a0 100644 --- a/sound/soc/samsung/snow.c +++ b/sound/soc/samsung/snow.c @@ -15,6 +15,7 @@ #include <linux/platform_device.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/mfd/syscon.h> #include <sound/soc.h> @@ -22,6 +23,10 @@ #define FIN_PLL_RATE 24000000 +#define XCLKOUT_OFFSET 0x0a00 +#define XCLKOUT_PARENT_MASK 0x1f00 +#define XCLKOUT_PARENT_XXTI 0x1000 + static struct snd_soc_dai_link snow_dai[] = { { .name = "Primary", @@ -66,6 +71,7 @@ static int snow_probe(struct platform_device *pdev) { struct snd_soc_card *card = &snow_snd; struct device_node *i2s_node, *codec_node; + struct regmap *pmureg; int i, ret; i2s_node = of_parse_phandle(pdev->dev.of_node, @@ -84,6 +90,19 @@ static int snow_probe(struct platform_device *pdev) return -EINVAL; } + /* + * On Snow board, SoC XCLKOUT line is connected to codec MCLK. + * One option would be to add another clock provider to configure + * XCLKOUT mux register. Else we can get pmu-syscon phandle from + * device tree and update the register here. + */ + pmureg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, + "samsung,pmu-syscon"); + if (!IS_ERR(pmureg)) { + regmap_update_bits(pmureg, XCLKOUT_OFFSET, + XCLKOUT_PARENT_MASK, XCLKOUT_PARENT_XXTI); + } + for (i = 0; i < ARRAY_SIZE(snow_dai); i++) { snow_dai[i].codec_of_node = codec_node; snow_dai[i].cpu_of_node = i2s_node;