[alsa-devel] [PATCH 00/11] enable imx6q_sabrelite sgtl5000 audio support
It's based on Dong Aisheng's early pinctrl patch and Sascha's v2 convert to common clk patch.
You can also get at: https://github.com/riczhao/kernel-imx/tree/topics/audio
Richard Zhao (11): dma: imx-sdma: make channel0 operations atomic ASoC: imx-sgtl5000: add of_node_put when probe fail. ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000 i2c: imx: add pinctrl support ARM: imx6q: move imx6q_sabrelite specific code to a dedicated function ARM: dts: imx6q-sabrelite: add ssi device ARM: dts: imx6q-sabrelite: add audmux device ASoC: imx-audmux: add pinctrl support ARM: imx6q: add ssi1 clk_lookup ARM: imx6q_sabrelite: clkdev_add cko1 for sgtl5000 ARM: dts: imx6q-sabrelite: add sound device imx6q-sabrelite-sgtl5000
arch/arm/boot/dts/imx6q-sabrelite.dts | 30 ++++++++++++++++ arch/arm/boot/dts/imx6q.dtsi | 46 +++++++++++++++++++++++-- arch/arm/mach-imx/clk-imx6q.c | 4 ++ arch/arm/mach-imx/mach-imx6q.c | 37 +++++++++++++++++++- drivers/dma/imx-sdma.c | 57 ++++++++++++++++++------------- drivers/i2c/busses/i2c-imx.c | 9 +++++ sound/soc/fsl/imx-audmux.c | 8 ++++ sound/soc/fsl/imx-sgtl5000.c | 61 +++++++++++++++++++++++++-------- 8 files changed, 208 insertions(+), 44 deletions(-)
Thanks Richard
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock. - Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de Signed-off-by: Richard Zhao richard.zhao@freescale.com --- drivers/dma/imx-sdma.c | 57 +++++++++++++++++++++++++++-------------------- 1 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index fddccae..fc49ffa 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,7 @@ #include <linux/mm.h> #include <linux/interrupt.h> #include <linux/clk.h> -#include <linux/wait.h> +#include <linux/delay.h> #include <linux/sched.h> #include <linux/semaphore.h> #include <linux/spinlock.h> @@ -324,7 +324,7 @@ struct sdma_engine { struct dma_device dma_device; struct clk *clk_ipg; struct clk *clk_ahb; - struct mutex channel_0_lock; + spinlock_t channel_0_lock; struct sdma_script_start_addrs *script_addrs; };
@@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel) }
/* - * sdma_run_channel - run a channel and wait till it's done + * sdma_run_channel0 - run a channel and wait till it's done */ -static int sdma_run_channel(struct sdma_channel *sdmac) +static int sdma_run_channel0(struct sdma_channel *sdmac) { struct sdma_engine *sdma = sdmac->sdma; int channel = sdmac->channel; int ret; + unsigned long timeout = 500;
- init_completion(&sdmac->done); - + if (channel) + return -EINVAL; sdma_enable_channel(sdma, channel);
- ret = wait_for_completion_timeout(&sdmac->done, HZ); + while (!(ret = readl_relaxed(sdma->regs + SDMA_H_INTR) & 1)) { + if (timeout-- <= 0) + break; + udelay(1); + } + + if (ret) { + /* Clear the interrupt status */ + writel_relaxed(ret, sdma->regs + SDMA_H_INTR); + } else { + dev_err(sdma->dev, "Timeout waiting for CH0 ready\n"); + }
return ret ? 0 : -ETIMEDOUT; } @@ -426,17 +438,17 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size, void *buf_virt; dma_addr_t buf_phys; int ret; - - mutex_lock(&sdma->channel_0_lock); + unsigned long flags;
buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL); if (!buf_virt) { - ret = -ENOMEM; - goto err_out; + return -ENOMEM; }
+ spin_lock_irqsave(&sdma->channel_0_lock, flags); + bd0->mode.command = C0_SETPM; bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD; bd0->mode.count = size / 2; @@ -445,12 +457,11 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
memcpy(buf_virt, buf, size);
- ret = sdma_run_channel(&sdma->channel[0]); + ret = sdma_run_channel0(&sdma->channel[0]);
- dma_free_coherent(NULL, size, buf_virt, buf_phys); + spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
-err_out: - mutex_unlock(&sdma->channel_0_lock); + dma_free_coherent(NULL, size, buf_virt, buf_phys);
return ret; } @@ -539,10 +550,6 @@ static void mxc_sdma_handle_channel(struct sdma_channel *sdmac) { complete(&sdmac->done);
- /* not interested in channel 0 interrupts */ - if (sdmac->channel == 0) - return; - if (sdmac->flags & IMX_DMA_SG_LOOP) sdma_handle_channel_loop(sdmac); else @@ -555,6 +562,8 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id) unsigned long stat;
stat = readl_relaxed(sdma->regs + SDMA_H_INTR); + /* not interested in channel 0 interrupts */ + stat &= ~1; writel_relaxed(stat, sdma->regs + SDMA_H_INTR);
while (stat) { @@ -660,6 +669,7 @@ static int sdma_load_context(struct sdma_channel *sdmac) struct sdma_context_data *context = sdma->context; struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd; int ret; + unsigned long flags;
if (sdmac->direction == DMA_DEV_TO_MEM) { load_address = sdmac->pc_from_device; @@ -677,7 +687,7 @@ static int sdma_load_context(struct sdma_channel *sdmac) dev_dbg(sdma->dev, "event_mask0 = 0x%08x\n", (u32)sdmac->event_mask[0]); dev_dbg(sdma->dev, "event_mask1 = 0x%08x\n", (u32)sdmac->event_mask[1]);
- mutex_lock(&sdma->channel_0_lock); + spin_lock_irqsave(&sdma->channel_0_lock, flags);
memset(context, 0, sizeof(*context)); context->channel_state.pc = load_address; @@ -696,10 +706,9 @@ static int sdma_load_context(struct sdma_channel *sdmac) bd0->mode.count = sizeof(*context) / 4; bd0->buffer_addr = sdma->context_phys; bd0->ext_buffer_addr = 2048 + (sizeof(*context) / 4) * channel; + ret = sdma_run_channel0(&sdma->channel[0]);
- ret = sdma_run_channel(&sdma->channel[0]); - - mutex_unlock(&sdma->channel_0_lock); + spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
return ret; } @@ -1305,7 +1314,7 @@ static int __init sdma_probe(struct platform_device *pdev) if (!sdma) return -ENOMEM;
- mutex_init(&sdma->channel_0_lock); + spin_lock_init(&sdma->channel_0_lock);
sdma->dev = &pdev->dev;
On Fri, Apr 27, 2012 at 03:02:55PM +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock.
- Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de Signed-off-by: Richard Zhao richard.zhao@freescale.com
drivers/dma/imx-sdma.c | 57 +++++++++++++++++++++++++++-------------------- 1 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index fddccae..fc49ffa 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,7 @@ #include <linux/mm.h> #include <linux/interrupt.h> #include <linux/clk.h> -#include <linux/wait.h> +#include <linux/delay.h> #include <linux/sched.h> #include <linux/semaphore.h> #include <linux/spinlock.h> @@ -324,7 +324,7 @@ struct sdma_engine { struct dma_device dma_device; struct clk *clk_ipg; struct clk *clk_ahb;
- struct mutex channel_0_lock;
- spinlock_t channel_0_lock; struct sdma_script_start_addrs *script_addrs;
};
@@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel) }
/*
- sdma_run_channel - run a channel and wait till it's done
*/
- sdma_run_channel0 - run a channel and wait till it's done
-static int sdma_run_channel(struct sdma_channel *sdmac) +static int sdma_run_channel0(struct sdma_channel *sdmac)
Renaming this to sdma_run_channel0 is fine, but then the argument should be changed to struct sdma_engine. It makes no sense to say in the function name that this function is channel 0 only and at the same time allow to pass in an arbitrary other channel.
Sascha
On Fri, Apr 27, 2012 at 09:55:44AM +0200, Sascha Hauer wrote:
On Fri, Apr 27, 2012 at 03:02:55PM +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock.
- Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de Signed-off-by: Richard Zhao richard.zhao@freescale.com
drivers/dma/imx-sdma.c | 57 +++++++++++++++++++++++++++-------------------- 1 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index fddccae..fc49ffa 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,7 @@ #include <linux/mm.h> #include <linux/interrupt.h> #include <linux/clk.h> -#include <linux/wait.h> +#include <linux/delay.h> #include <linux/sched.h> #include <linux/semaphore.h> #include <linux/spinlock.h> @@ -324,7 +324,7 @@ struct sdma_engine { struct dma_device dma_device; struct clk *clk_ipg; struct clk *clk_ahb;
- struct mutex channel_0_lock;
- spinlock_t channel_0_lock; struct sdma_script_start_addrs *script_addrs;
};
@@ -402,19 +402,31 @@ static void sdma_enable_channel(struct sdma_engine *sdma, int channel) }
/*
- sdma_run_channel - run a channel and wait till it's done
*/
- sdma_run_channel0 - run a channel and wait till it's done
-static int sdma_run_channel(struct sdma_channel *sdmac) +static int sdma_run_channel0(struct sdma_channel *sdmac)
Renaming this to sdma_run_channel0 is fine, but then the argument should be changed to struct sdma_engine. It makes no sense to say in the function name that this function is channel 0 only and at the same time allow to pass in an arbitrary other channel.
Correct. Thanks.
Richard
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
No this is wrong behavior. You should not call dma prepare functions in any of the sound trigger calls. It would make sense to move this in sound prepare callback.
On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
No this is wrong behavior. You should not call dma prepare functions in any of the sound trigger calls. It would make sense to move this in sound prepare callback.
Then, could you please doc it somewhere? I think I'm not the only one confused.
Thanks Richard
-- ~Vinod
On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:
On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
No this is wrong behavior. You should not call dma prepare functions in any of the sound trigger calls. It would make sense to move this in sound prepare callback.
Then, could you please doc it somewhere? I think I'm not the only one confused.
See the soc-dmaengine.c for correct behavior! I can document only dmaengine behavior (which is already there) and not how each subsystem should use this. People is subsystems need to see how to use the dmaengine APIs sanely
On Fri, Apr 27, 2012 at 03:52:10PM +0530, Vinod Koul wrote:
On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:
Then, could you please doc it somewhere? I think I'm not the only one confused.
See the soc-dmaengine.c for correct behavior!
Better yet, convert your code to use it if it's not doing so already!
On Fri, Apr 27, 2012 at 03:52:10PM +0530, Vinod Koul wrote:
On Fri, 2012-04-27 at 16:41 +0800, Richard Zhao wrote:
On Fri, Apr 27, 2012 at 01:51:40PM +0530, Vinod Koul wrote:
On Fri, 2012-04-27 at 15:02 +0800, Richard Zhao wrote:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
No this is wrong behavior. You should not call dma prepare functions in any of the sound trigger calls. It would make sense to move this in sound prepare callback.
Then, could you please doc it somewhere? I think I'm not the only one confused.
See the soc-dmaengine.c for correct behavior!
Do you mean sound/soc/soc-dmaengine-pcm.c ? It's where it calls dma prepare in trigger function. snd_dmaengine_pcm_trigger --> dmaengine_pcm_prepare_and_submit --> dmaengine_prep_dma_cyclic
I can document only dmaengine behavior (which is already there)
Sure, I mean, can you doc in include/linux/dmaengine.h that dmaengine_prep_xxx may sleep?
and not how each subsystem should use this. People is subsystems need to see how to use the dmaengine APIs sanely
Thanks Richard
-- ~Vinod
On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
Sure, I mean, can you doc in include/linux/dmaengine.h that dmaengine_prep_xxx may sleep?
Incorrect. They may _not_ sleep.
But I have seen that we are using the kzalloc in the dmaengine_prep_xxx and kzalloc is sleeping call.
On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
Sure, I mean, can you doc in include/linux/dmaengine.h that dmaengine_prep_xxx may sleep?
Incorrect. They may _not_ sleep.
But I have seen that we are using the kzalloc in the dmaengine_prep_xxx and kzalloc is sleeping call.
Not with GFP_ATOMIC it isn't.
On Friday 27 April 2012 06:40 PM, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
Sure, I mean, can you doc in include/linux/dmaengine.h that dmaengine_prep_xxx may sleep?
Incorrect. They may _not_ sleep.
But I have seen that we are using the kzalloc in the dmaengine_prep_xxx and kzalloc is sleeping call.
Not with GFP_ATOMIC it isn't.
Yes, with GFP_ATOMIC , it will not be a sleeping calls. I will fix my dma driver accordingly.
On Fri, Apr 27, 2012 at 02:10:10PM +0100, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 06:31:13PM +0530, Laxman Dewangan wrote:
On Friday 27 April 2012 05:03 PM, Russell King - ARM Linux wrote:
On Fri, Apr 27, 2012 at 07:26:56PM +0800, Richard Zhao wrote:
Sure, I mean, can you doc in include/linux/dmaengine.h that dmaengine_prep_xxx may sleep?
Incorrect. They may _not_ sleep.
Vinod, do you agree? Any way, better doc it.
Thanks Richard
But I have seen that we are using the kzalloc in the dmaengine_prep_xxx and kzalloc is sleeping call.
Not with GFP_ATOMIC it isn't.
Hi,
Richard Zhao writes:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock.
- Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de
Actually I didn't sign off the patch that I posted, because I wanted to wait for more comments first.
Lothar Waßmann
On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
Hi,
Richard Zhao writes:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock.
- Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de
Actually I didn't sign off the patch that I posted, because I wanted to wait for more comments first.
I send it out with slight modifications because the series highly depend on it. Will you take it over or let me put it in next version? Both are ok to me.
Thanks Richard
Lothar Waßmann
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________
Hi,
Richard Zhao writes:
On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
Hi,
Richard Zhao writes:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock.
- Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de
Actually I didn't sign off the patch that I posted, because I wanted to wait for more comments first.
I send it out with slight modifications because the series highly depend on it. Will you take it over or let me put it in next version? Both are ok to me.
I think you should keep it as part of your sound patches and I will test your final version on our hardware.
Lothar Waßmann
On Fri, Apr 27, 2012 at 11:13 AM, Lothar Waßmann LW@karo-electronics.de wrote:
Hi,
Richard Zhao writes:
On Fri, Apr 27, 2012 at 11:18:31AM +0200, Lothar Waßmann wrote:
Hi,
Richard Zhao writes:
device_prep_dma_cyclic may be call in audio trigger function which is atomic context, so we make it atomic too.
- change channel0 lock to spinlock. - Use polling to wait for channel0 finish running.
Signed-off-by: Lothar Waßmann LW@KARO-electronics.de
Actually I didn't sign off the patch that I posted, because I wanted to wait for more comments first.
I send it out with slight modifications because the series highly depend on it. Will you take it over or let me put it in next version? Both are ok to me.
I think you should keep it as part of your sound patches and I will test your final version on our hardware.
I hope we can get a conclusion that the prep_slave_sg() can be called in atomic context or not. My patch "add DMA support to UART" heavily depends on it.
Huang Shijie
Lothar Waßmann
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Geschäftsführer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de ___________________________________________________________
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- sound/soc/fsl/imx-sgtl5000.c | 29 ++++++++++++++++++----------- 1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index e1a7441..73b935e 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -103,24 +103,28 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0); if (!ssi_np || !codec_np) { dev_err(&pdev->dev, "phandle missing or invalid\n"); - return -EINVAL; + ret = -EINVAL; + goto fail; }
ssi_pdev = of_find_device_by_node(ssi_np); if (!ssi_pdev) { dev_err(&pdev->dev, "failed to find SSI platform device\n"); - return -EINVAL; + ret = -EINVAL; + goto fail; }
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); - if (!data) - return -ENOMEM; + if (!data) { + ret = -ENOMEM; + goto fail; + }
ret = of_property_read_u32(codec_np, "clock-frequency", &data->clk_frequency); if (ret) { dev_err(&pdev->dev, "clock-frequency missing or invalid\n"); - return ret; + goto fail; }
data->dai.name = "HiFi"; @@ -136,10 +140,10 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) data->card.dev = &pdev->dev; ret = snd_soc_of_parse_card_name(&data->card, "model"); if (ret) - return ret; + goto fail; ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing"); if (ret) - return ret; + goto fail; data->card.num_links = 1; data->card.dai_link = &data->dai; data->card.dapm_widgets = imx_sgtl5000_dapm_widgets; @@ -148,14 +152,17 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) ret = snd_soc_register_card(&data->card); if (ret) { dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret); - return ret; + goto fail; }
platform_set_drvdata(pdev, data); - of_node_put(ssi_np); - of_node_put(codec_np); +fail: + if (ssi_np) + of_node_put(ssi_np); + if (codec_np) + of_node_put(codec_np);
- return 0; + return ret; }
static int __devexit imx_sgtl5000_remove(struct platform_device *pdev)
On Fri, Apr 27, 2012 at 03:02:56PM +0800, Richard Zhao wrote:
Signed-off-by: Richard Zhao richard.zhao@freescale.com
Applied, thanks.
From: Richard Zhao richard.zhao@linaro.org
It tries to clk_get the clock. And if it failed, it assumes the clock by default enabled.
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- sound/soc/fsl/imx-sgtl5000.c | 40 ++++++++++++++++++++++++++++++++-------- 1 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 73b935e..3a729ca 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -13,6 +13,8 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/of_i2c.h> +#include <linux/clk.h> #include <sound/soc.h>
#include "../codecs/sgtl5000.h" @@ -25,6 +27,7 @@ struct imx_sgtl5000_data { struct snd_soc_card card; char codec_dai_name[DAI_NAME_SIZE]; char platform_name[DAI_NAME_SIZE]; + struct clk *codec_clk; unsigned int clk_frequency; };
@@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct device_node *ssi_np, *codec_np; struct platform_device *ssi_pdev; + struct i2c_client *codec_dev; struct imx_sgtl5000_data *data; int int_port, ext_port; int ret; @@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) ret = -EINVAL; goto fail; } + codec_dev = of_find_i2c_device_by_node(codec_np); + if (!codec_dev) { + dev_err(&pdev->dev, "failed to find codec platform device\n"); + return -EINVAL; + }
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) { @@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- ret = of_property_read_u32(codec_np, "clock-frequency", - &data->clk_frequency); - if (ret) { - dev_err(&pdev->dev, "clock-frequency missing or invalid\n"); - goto fail; + data->codec_clk = clk_get(&codec_dev->dev, NULL); + if (IS_ERR(data->codec_clk)) { + /* assuming clock enabled by default */ + data->codec_clk = NULL; + ret = of_property_read_u32(codec_np, "clock-frequency", + &data->clk_frequency); + if (ret) { + dev_err(&codec_dev->dev, + "clock-frequency missing or invalid\n"); + goto fail; + } + } else { + data->clk_frequency = clk_get_rate(data->codec_clk); + clk_prepare_enable(data->codec_clk); }
data->dai.name = "HiFi"; @@ -140,10 +158,10 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) data->card.dev = &pdev->dev; ret = snd_soc_of_parse_card_name(&data->card, "model"); if (ret) - goto fail; + goto clk_fail; ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing"); if (ret) - goto fail; + goto clk_fail; data->card.num_links = 1; data->card.dai_link = &data->dai; data->card.dapm_widgets = imx_sgtl5000_dapm_widgets; @@ -152,10 +170,12 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) ret = snd_soc_register_card(&data->card); if (ret) { dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret); - goto fail; + goto clk_fail; }
platform_set_drvdata(pdev, data); +clk_fail: + clk_put(data->codec_clk); fail: if (ssi_np) of_node_put(ssi_np); @@ -169,6 +189,10 @@ static int __devexit imx_sgtl5000_remove(struct platform_device *pdev) { struct imx_sgtl5000_data *data = platform_get_drvdata(pdev);
+ if (data->codec_clk) { + clk_disable_unprepare(data->codec_clk); + clk_put(data->codec_clk); + } snd_soc_unregister_card(&data->card);
return 0;
On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
It tries to clk_get the clock. And if it failed, it assumes the clock by default enabled.
Applied, thanks.
On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
It tries to clk_get the clock. And if it failed, it assumes the clock by default enabled.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
sound/soc/fsl/imx-sgtl5000.c | 40 ++++++++++++++++++++++++++++++++-------- 1 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 73b935e..3a729ca 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -13,6 +13,8 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/of_i2c.h> +#include <linux/clk.h> #include <sound/soc.h>
#include "../codecs/sgtl5000.h" @@ -25,6 +27,7 @@ struct imx_sgtl5000_data { struct snd_soc_card card; char codec_dai_name[DAI_NAME_SIZE]; char platform_name[DAI_NAME_SIZE];
- struct clk *codec_clk; unsigned int clk_frequency;
};
@@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct device_node *ssi_np, *codec_np; struct platform_device *ssi_pdev;
- struct i2c_client *codec_dev; struct imx_sgtl5000_data *data; int int_port, ext_port; int ret;
@@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) ret = -EINVAL; goto fail; }
- codec_dev = of_find_i2c_device_by_node(codec_np);
- if (!codec_dev) {
dev_err(&pdev->dev, "failed to find codec platform device\n");return -EINVAL;- }
What if the codec is accessed via SPI bus on some machines?
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) { @@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- ret = of_property_read_u32(codec_np, "clock-frequency",
&data->clk_frequency);- if (ret) {
dev_err(&pdev->dev, "clock-frequency missing or invalid\n");goto fail;
- data->codec_clk = clk_get(&codec_dev->dev, NULL);
It's a clock of sgtl5000 codec. I feel it makes more sense to have sgtl5000 device driver than machine driver to manage this clock.
Regards, Shawn
- if (IS_ERR(data->codec_clk)) {
/* assuming clock enabled by default */data->codec_clk = NULL;ret = of_property_read_u32(codec_np, "clock-frequency",&data->clk_frequency);if (ret) {dev_err(&codec_dev->dev,"clock-frequency missing or invalid\n");goto fail;}- } else {
data->clk_frequency = clk_get_rate(data->codec_clk);clk_prepare_enable(data->codec_clk);
On Tue, May 01, 2012 at 09:44:48PM +0800, Shawn Guo wrote:
On Fri, Apr 27, 2012 at 03:02:57PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
It tries to clk_get the clock. And if it failed, it assumes the clock by default enabled.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
sound/soc/fsl/imx-sgtl5000.c | 40 ++++++++++++++++++++++++++++++++-------- 1 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 73b935e..3a729ca 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -13,6 +13,8 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/of_i2c.h> +#include <linux/clk.h> #include <sound/soc.h>
#include "../codecs/sgtl5000.h" @@ -25,6 +27,7 @@ struct imx_sgtl5000_data { struct snd_soc_card card; char codec_dai_name[DAI_NAME_SIZE]; char platform_name[DAI_NAME_SIZE];
- struct clk *codec_clk; unsigned int clk_frequency;
};
@@ -58,6 +61,7 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; struct device_node *ssi_np, *codec_np; struct platform_device *ssi_pdev;
- struct i2c_client *codec_dev; struct imx_sgtl5000_data *data; int int_port, ext_port; int ret;
@@ -113,6 +117,11 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) ret = -EINVAL; goto fail; }
- codec_dev = of_find_i2c_device_by_node(codec_np);
- if (!codec_dev) {
dev_err(&pdev->dev, "failed to find codec platform device\n");return -EINVAL;- }
What if the codec is accessed via SPI bus on some machines?
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); if (!data) { @@ -120,11 +129,20 @@ static int __devinit imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- ret = of_property_read_u32(codec_np, "clock-frequency",
&data->clk_frequency);- if (ret) {
dev_err(&pdev->dev, "clock-frequency missing or invalid\n");goto fail;
- data->codec_clk = clk_get(&codec_dev->dev, NULL);
It's a clock of sgtl5000 codec. I feel it makes more sense to have sgtl5000 device driver than machine driver to manage this clock.
The ASoC machine driver handled the clock settings, I just followed the same way, using snd_soc_dai_set_sysclk to set codec clk. But what you said is reasonable. I also find it's impossible to use devm_get_clk.
Mark, do you suggest get clk in ASoC machine driver or in codec driver?
Thanks Richard
Regards, Shawn
- if (IS_ERR(data->codec_clk)) {
/* assuming clock enabled by default */data->codec_clk = NULL;ret = of_property_read_u32(codec_np, "clock-frequency",&data->clk_frequency);if (ret) {dev_err(&codec_dev->dev,"clock-frequency missing or invalid\n");goto fail;}- } else {
data->clk_frequency = clk_get_rate(data->codec_clk);clk_prepare_enable(data->codec_clk);
On Wed, May 02, 2012 at 06:50:24PM +0800, Richard Zhao wrote:
On Tue, May 01, 2012 at 09:44:48PM +0800, Shawn Guo wrote:
- data->codec_clk = clk_get(&codec_dev->dev, NULL);
It's a clock of sgtl5000 codec. I feel it makes more sense to have sgtl5000 device driver than machine driver to manage this clock.
The ASoC machine driver handled the clock settings, I just followed the same way, using snd_soc_dai_set_sysclk to set codec clk. But what you said is reasonable. I also find it's impossible to use devm_get_clk.
Mark, do you suggest get clk in ASoC machine driver or in codec driver?
We generally try to keep the clock API out of CODEC drivers due to the issues with the clock API that the generic implementation is intended to help us solve.
The major problem here is that we can't rely on having the clock API present at all on most platforms so as soon as we start adding clock support into things we have to do a performance with HAVE_CLK which is at best annoying.
There's also the fact that even if the clock API is available we've no way to register a clock consistently and since the audio clock tree often needs management off-SoC for at least some components when the clock API is in use you end up having to handle clock trees that go into and out of the clock API.
The upshot of this is that while we really should be using the clock API for clocks the API as it stands is somewhat difficult to deploy in cross platform code. What I'd suggest is connecting the clock to the CODEC in the device tree but actually doing the clk_get() in the machine driver.
On Wed, May 02, 2012 at 12:41:22PM +0100, Mark Brown wrote:
We generally try to keep the clock API out of CODEC drivers due to the issues with the clock API that the generic implementation is intended to help us solve.
The major problem here is that we can't rely on having the clock API present at all on most platforms so as soon as we start adding clock support into things we have to do a performance with HAVE_CLK which is at best annoying.
Is this something that the series below tries to solve?
http://thread.gmane.org/gmane.linux.ports.arm.kernel/163652
Regards, Shawn
There's also the fact that even if the clock API is available we've no way to register a clock consistently and since the audio clock tree often needs management off-SoC for at least some components when the clock API is in use you end up having to handle clock trees that go into and out of the clock API.
The upshot of this is that while we really should be using the clock API for clocks the API as it stands is somewhat difficult to deploy in cross platform code. What I'd suggest is connecting the clock to the CODEC in the device tree but actually doing the clk_get() in the machine driver.
On Wed, May 02, 2012 at 08:12:24PM +0800, Shawn Guo wrote:
On Wed, May 02, 2012 at 12:41:22PM +0100, Mark Brown wrote:
The major problem here is that we can't rely on having the clock API present at all on most platforms so as soon as we start adding clock support into things we have to do a performance with HAVE_CLK which is at best annoying.
Is this something that the series below tries to solve?
Well, it helps building but that's only part of the issue - we've still got registration problems and if we were using the API in anger we would need things like clk_set_rate() to actually work. It's not just a case of simply enabling and disabling clocks (which can be stubbed out pretty effectively), audio generally needs to manage the clock rates too.
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/boot/dts/imx6q-sabrelite.dts | 2 ++ arch/arm/boot/dts/imx6q.dtsi | 16 ++++++++++++++++ drivers/i2c/busses/i2c-imx.c | 9 +++++++++ 3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 4663a4e..4e13293 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -50,6 +50,8 @@ i2c@021a0000 { /* I2C1 */ status = "okay"; clock-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c1_1>;
codec: sgtl5000@0a { compatible = "fsl,sgtl5000"; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 2ba32e7..fe8c80d 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -390,6 +390,22 @@ reg = <0x020e0000 0x4000>;
/* shared pinctrl settings */ + i2c1 { + pinctrl_i2c1_1: i2c1grp-1 { + fsl,pins = "MX6Q_PAD_EIM_D21", + "MX6Q_PAD_EIM_D28"; + fsl,hysteresis = <1>; + fsl,mux = <0x16 0x11>; + fsl,pull = <2>; + fsl,pue = <1>; + fsl,pke = <1>; + fsl,open-drain = <1>; + fsl,speed = <2>; + fsl,drive-strength = <6>; + fsl,slew-rate = <1>; + }; + }; + uart4 { pinctrl_uart4_1: uart4grp-1 { fsl,pins = "MX6Q_PAD_KEY_COL0", diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index dfb84b7..7a52067 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -48,6 +48,7 @@ #include <linux/platform_device.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/pinctrl/consumer.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_i2c.h> @@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev) struct imx_i2c_struct *i2c_imx; struct resource *res; struct imxi2c_platform_data *pdata = pdev->dev.platform_data; + struct pinctrl *pct; void __iomem *base; resource_size_t res_size; int irq, bitrate; @@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev) i2c_imx->base = base; i2c_imx->res = res;
+ pct = devm_pinctrl_get_select_default(&pdev->dev); + if (IS_ERR(pct)) { + dev_err(&pdev->dev, "can't get/select pinctrl\n"); + ret = PTR_ERR(pct); + goto fail3; + } + /* Get I2C clock */ i2c_imx->clk = clk_get(&pdev->dev, "i2c_clk"); if (IS_ERR(i2c_imx->clk)) {
On Fri, Apr 27, 2012 at 03:02:58PM +0800, Richard Zhao wrote:
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/boot/dts/imx6q-sabrelite.dts | 2 ++ arch/arm/boot/dts/imx6q.dtsi | 16 ++++++++++++++++ drivers/i2c/busses/i2c-imx.c | 9 +++++++++ 3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 4663a4e..4e13293 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -50,6 +50,8 @@ i2c@021a0000 { /* I2C1 */ status = "okay"; clock-frequency = <100000>;
pinctrl-names = "default";pinctrl-0 = <&pinctrl_i2c1_1>; codec: sgtl5000@0a { compatible = "fsl,sgtl5000";diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 2ba32e7..fe8c80d 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -390,6 +390,22 @@ reg = <0x020e0000 0x4000>;
/* shared pinctrl settings */
i2c1 {pinctrl_i2c1_1: i2c1grp-1 {fsl,pins = "MX6Q_PAD_EIM_D21","MX6Q_PAD_EIM_D28";fsl,hysteresis = <1>;fsl,mux = <0x16 0x11>;fsl,pull = <2>;fsl,pue = <1>;fsl,pke = <1>;fsl,open-drain = <1>;fsl,speed = <2>;fsl,drive-strength = <6>;fsl,slew-rate = <1>;};
The pinctrl binding is changed a bit since v1. You may need to change here according to v2 or latter.
};uart4 { pinctrl_uart4_1: uart4grp-1 { fsl,pins = "MX6Q_PAD_KEY_COL0",diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index dfb84b7..7a52067 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -48,6 +48,7 @@ #include <linux/platform_device.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/pinctrl/consumer.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_i2c.h> @@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev) struct imx_i2c_struct *i2c_imx; struct resource *res; struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
- struct pinctrl *pct; void __iomem *base; resource_size_t res_size; int irq, bitrate;
@@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev) i2c_imx->base = base; i2c_imx->res = res;
- pct = devm_pinctrl_get_select_default(&pdev->dev);
You may want to check this change will break other platforms also using this driver. Refer to: http://www.spinics.net/lists/arm-kernel/msg171538.html
Regards Dong Aisheng
On Fri, Apr 27, 2012 at 04:40:57PM +0800, Dong Aisheng wrote:
On Fri, Apr 27, 2012 at 03:02:58PM +0800, Richard Zhao wrote:
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/boot/dts/imx6q-sabrelite.dts | 2 ++ arch/arm/boot/dts/imx6q.dtsi | 16 ++++++++++++++++ drivers/i2c/busses/i2c-imx.c | 9 +++++++++ 3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 4663a4e..4e13293 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -50,6 +50,8 @@ i2c@021a0000 { /* I2C1 */ status = "okay"; clock-frequency = <100000>;
pinctrl-names = "default";pinctrl-0 = <&pinctrl_i2c1_1>; codec: sgtl5000@0a { compatible = "fsl,sgtl5000";diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 2ba32e7..fe8c80d 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -390,6 +390,22 @@ reg = <0x020e0000 0x4000>;
/* shared pinctrl settings */
i2c1 {pinctrl_i2c1_1: i2c1grp-1 {fsl,pins = "MX6Q_PAD_EIM_D21","MX6Q_PAD_EIM_D28";fsl,hysteresis = <1>;fsl,mux = <0x16 0x11>;fsl,pull = <2>;fsl,pue = <1>;fsl,pke = <1>;fsl,open-drain = <1>;fsl,speed = <2>;fsl,drive-strength = <6>;fsl,slew-rate = <1>;};The pinctrl binding is changed a bit since v1. You may need to change here according to v2 or latter.
Yes, I'll try your v3 with SION.
};uart4 { pinctrl_uart4_1: uart4grp-1 { fsl,pins = "MX6Q_PAD_KEY_COL0",diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index dfb84b7..7a52067 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -48,6 +48,7 @@ #include <linux/platform_device.h> #include <linux/clk.h> #include <linux/slab.h> +#include <linux/pinctrl/consumer.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/of_i2c.h> @@ -470,6 +471,7 @@ static int __init i2c_imx_probe(struct platform_device *pdev) struct imx_i2c_struct *i2c_imx; struct resource *res; struct imxi2c_platform_data *pdata = pdev->dev.platform_data;
- struct pinctrl *pct; void __iomem *base; resource_size_t res_size; int irq, bitrate;
@@ -520,6 +522,13 @@ static int __init i2c_imx_probe(struct platform_device *pdev) i2c_imx->base = base; i2c_imx->res = res;
- pct = devm_pinctrl_get_select_default(&pdev->dev);
You may want to check this change will break other platforms also using this driver. Refer to: http://www.spinics.net/lists/arm-kernel/msg171538.html
I didn't expect it can go in right now. It surely depends on your dummy pinctrl.
Thanks Richard
Regards Dong Aisheng
It'll be easier to add other board specific code.
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/mach-imx/mach-imx6q.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index da6c1d9..d25c5d8 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -75,11 +75,16 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev) return 0; }
+static void __init imx6q_sabrelite_init(void) +{ + phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, + ksz9021rn_phy_fixup); +} + static void __init imx6q_init_machine(void) { if (of_machine_is_compatible("fsl,imx6q-sabrelite")) - phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, - ksz9021rn_phy_fixup); + imx6q_sabrelite_init();
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
On Fri, Apr 27, 2012 at 03:02:59PM +0800, Richard Zhao wrote:
It'll be easier to add other board specific code.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
Applied, thanks.
Regards, Shawn
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/boot/dts/imx6q-sabrelite.dts | 9 +++++++++ arch/arm/boot/dts/imx6q.dtsi | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 4e13293..7bd8855 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -22,6 +22,15 @@ };
soc { + aips-bus@02000000 { /* AIPS1 */ + spba-bus@02000000 { + ssi1: ssi@02028000 { + fsl,mode = "i2s-slave"; + status = "okay"; + }; + }; + }; + aips-bus@02100000 { /* AIPS2 */ enet@02188000 { phy-mode = "rgmii"; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index fe8c80d..da42fc0 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -177,19 +177,31 @@ interrupts = <0 51 0x04>; };
- ssi@02028000 { /* SSI1 */ + ssi1: ssi@02028000 { + compatible = "fsl,imx6q-ssi","fsl,imx21-ssi"; reg = <0x02028000 0x4000>; interrupts = <0 46 0x04>; + fsl,fifo-depth = <15>; + fsl,ssi-dma-events = <38 37>; + status = "disabled"; };
- ssi@0202c000 { /* SSI2 */ + ssi2: ssi@0202c000 { + compatible = "fsl,imx6q-ssi","fsl,imx21-ssi"; reg = <0x0202c000 0x4000>; interrupts = <0 47 0x04>; + fsl,fifo-depth = <15>; + fsl,ssi-dma-events = <42 41>; + status = "disabled"; };
- ssi@02030000 { /* SSI3 */ + ssi3: ssi@02030000 { + compatible = "fsl,imx6q-ssi","fsl,imx21-ssi"; reg = <0x02030000 0x4000>; interrupts = <0 48 0x04>; + fsl,fifo-depth = <15>; + fsl,ssi-dma-events = <46 45>; + status = "disabled"; };
asrc@02034000 {
On Fri, Apr 27, 2012 at 03:03:00PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com
I tried to apply patch #6 and #7, but they did not apply.
On Tue, May 01, 2012 at 09:26:53PM +0800, Shawn Guo wrote:
On Fri, Apr 27, 2012 at 03:03:00PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com
I tried to apply patch #6 and #7, but they did not apply.
Just manually applied these two patches on imx/dt branch, please check if they are applied correctly.
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/boot/dts/imx6q-sabrelite.dts | 3 +++ arch/arm/boot/dts/imx6q.dtsi | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 7bd8855..02f93bc 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -52,6 +52,9 @@ status = "okay"; };
+ audmux@021d8000 { + status = "okay"; + }; uart2: uart@021e8000 { status = "okay"; }; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index da42fc0..7bf402e 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -596,7 +596,9 @@ };
audmux@021d8000 { + compatible = "fsl,imx6q-audmux", "fsl,imx31-audmux"; reg = <0x021d8000 0x4000>; + status = "disabled"; };
mipi@021dc000 { /* MIPI-CSI */
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/boot/dts/imx6q-sabrelite.dts | 3 +++ arch/arm/boot/dts/imx6q.dtsi | 10 ++++++++++ sound/soc/fsl/imx-audmux.c | 8 ++++++++ 3 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index 02f93bc..cdae2dd 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -54,7 +54,10 @@
audmux@021d8000 { status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_audmux_1>; }; + uart2: uart@021e8000 { status = "okay"; }; diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi index 7bf402e..3c3004d 100644 --- a/arch/arm/boot/dts/imx6q.dtsi +++ b/arch/arm/boot/dts/imx6q.dtsi @@ -402,6 +402,16 @@ reg = <0x020e0000 0x4000>;
/* shared pinctrl settings */ + audmux { + pinctrl_audmux_1: audmux-1 { + fsl,pins = "MX6Q_PAD_SD2_DAT0", + "MX6Q_PAD_SD2_DAT3", + "MX6Q_PAD_SD2_DAT2", + "MX6Q_PAD_SD2_DAT1"; + fsl,mux = <3 3 3 3>; + }; + }; + i2c1 { pinctrl_i2c1_1: i2c1grp-1 { fsl,pins = "MX6Q_PAD_EIM_D21", diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c index f237003..6c7dfc0 100644 --- a/sound/soc/fsl/imx-audmux.c +++ b/sound/soc/fsl/imx-audmux.c @@ -26,6 +26,7 @@ #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/slab.h> +#include <linux/pinctrl/consumer.h>
#include "imx-audmux.h"
@@ -249,6 +250,7 @@ EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port); static int __devinit imx_audmux_probe(struct platform_device *pdev) { struct resource *res; + struct pinctrl *pct; const struct of_device_id *of_id = of_match_device(imx_audmux_dt_ids, &pdev->dev);
@@ -257,6 +259,12 @@ static int __devinit imx_audmux_probe(struct platform_device *pdev) if (!audmux_base) return -EADDRNOTAVAIL;
+ pct = devm_pinctrl_get_select_default(&pdev->dev); + if (IS_ERR(pct)) { + dev_err(&pdev->dev, "setup pinctrl failed!"); + return PTR_ERR(pct); + } + audmux_clk = clk_get(&pdev->dev, "audmux"); if (IS_ERR(audmux_clk)) { dev_dbg(&pdev->dev, "cannot get clock: %ld\n",
It's used by audio drivers.
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/mach-imx/clk-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index f40a35d..9a03dcc 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma"); clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog"); + clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) { c = clk_get_sys(clks_init_on[i], NULL);
On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
It's used by audio drivers.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index f40a35d..9a03dcc 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma"); clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
- clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
The ssi clock needs a general cleanup on all i.MX just like I cleaned up the other units. The SSI unit has at least a register clock and a baud clock. What the driver requests and enables is the register clock.
The baud clock is currently unused and is needed only for master mode (which is not implemented in the ssi driver)
So where we want to come to is:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi"); clk_register_clkdev(clk[ssi1], "baud", "2028000.ssi");
Sascha
On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
It's used by audio drivers.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index f40a35d..9a03dcc 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma"); clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
- clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
The ssi clock needs a general cleanup on all i.MX just like I cleaned up the other units. The SSI unit has at least a register clock and a baud clock. What the driver requests and enables is the register clock.
The baud clock is currently unused and is needed only for master mode (which is not implemented in the ssi driver)
Are you sure for that? If I don't enable clk[ssi1], the ssi will not work.
So where we want to come to is:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.
Thanks Richard
clk_register_clkdev(clk[ssi1], "baud", "2028000.ssi");
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Apr 27, 2012 at 05:10:56PM +0800, Richard Zhao wrote:
On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
It's used by audio drivers.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index f40a35d..9a03dcc 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma"); clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
- clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
The ssi clock needs a general cleanup on all i.MX just like I cleaned up the other units. The SSI unit has at least a register clock and a baud clock. What the driver requests and enables is the register clock.
The baud clock is currently unused and is needed only for master mode (which is not implemented in the ssi driver)
Are you sure for that? If I don't enable clk[ssi1], the ssi will not work.
So where we want to come to is:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.
Can you please ask your IC guys for clarification?
For example on i.MX5 we have a ssi ipg clock and a ssi serial clock. Both can be gated with two individual gate bits.
The i.MX6 datasheet (and also several other i.MX datasheets) is quite nebulous. The i.MX6 has only one gate bit for each SSI unit, but it's not clear if this bit actually gates both the ipg and serial clock or only one of them.
My general idea is that each unit in the SoC has different input clocks. In the driver we need to clk_get() the input clocks. If a given SoC has no software control over some of the devices input clocks, then we need to provide a dummy for this, because other SoCs have control over the clock.
Sascha
On Fri, Apr 27, 2012 at 11:25:29AM +0200, Sascha Hauer wrote:
On Fri, Apr 27, 2012 at 05:10:56PM +0800, Richard Zhao wrote:
On Fri, Apr 27, 2012 at 10:04:12AM +0200, Sascha Hauer wrote:
On Fri, Apr 27, 2012 at 03:03:03PM +0800, Richard Zhao wrote:
It's used by audio drivers.
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index f40a35d..9a03dcc 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -418,6 +418,7 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[sdma], NULL, "20ec000.sdma"); clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog");
- clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
The ssi clock needs a general cleanup on all i.MX just like I cleaned up the other units. The SSI unit has at least a register clock and a baud clock. What the driver requests and enables is the register clock.
The baud clock is currently unused and is needed only for master mode (which is not implemented in the ssi driver)
Are you sure for that? If I don't enable clk[ssi1], the ssi will not work.
So where we want to come to is:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.
Can you please ask your IC guys for clarification?
For example on i.MX5 we have a ssi ipg clock and a ssi serial clock. Both can be gated with two individual gate bits.
The i.MX6 datasheet (and also several other i.MX datasheets) is quite nebulous. The i.MX6 has only one gate bit for each SSI unit, but it's not clear if this bit actually gates both the ipg and serial clock or only one of them.
You're right. ipg and serial clocks share the same gate. How do we handle it? I think it's not the only one and won't be last one.
My general idea is that each unit in the SoC has different input clocks. In the driver we need to clk_get() the input clocks. If a given SoC has no software control over some of the devices input clocks, then we need to provide a dummy for this, because other SoCs have control over the clock.
Great.
Thanks Richard
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Apr 30, 2012 at 10:01:46AM +0800, Richard Zhao wrote:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.
Can you please ask your IC guys for clarification?
For example on i.MX5 we have a ssi ipg clock and a ssi serial clock. Both can be gated with two individual gate bits.
The i.MX6 datasheet (and also several other i.MX datasheets) is quite nebulous. The i.MX6 has only one gate bit for each SSI unit, but it's not clear if this bit actually gates both the ipg and serial clock or only one of them.
You're right. ipg and serial clocks share the same gate. How do we handle it? I think it's not the only one and won't be last one.
We don't have support for a single gate gating two clocks right now and I don't know how this fits into the clock framework. We could pretend that only the ipg clock is gateable because this is needed anyway when the SSI unit is used.
It seems we need a proper solution for this later.
Sascha
On Mon, Apr 30, 2012 at 02:18:57PM +0200, Sascha Hauer wrote:
On Mon, Apr 30, 2012 at 10:01:46AM +0800, Richard Zhao wrote:
clk_register_clkdev(clk[ipg], "ipg", "2028000.ssi");
ssi don't have ipg gate. We can let it always on for imx6q.
Can you please ask your IC guys for clarification?
For example on i.MX5 we have a ssi ipg clock and a ssi serial clock. Both can be gated with two individual gate bits.
The i.MX6 datasheet (and also several other i.MX datasheets) is quite nebulous. The i.MX6 has only one gate bit for each SSI unit, but it's not clear if this bit actually gates both the ipg and serial clock or only one of them.
You're right. ipg and serial clocks share the same gate. How do we handle it? I think it's not the only one and won't be last one.
We don't have support for a single gate gating two clocks right now and I don't know how this fits into the clock framework. We could pretend that only the ipg clock is gateable because this is needed anyway when the SSI unit is used.
Shawn, will you add clk ssi_ipg?
Thanks Richard
It seems we need a proper solution for this later.
Sascha
-- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, May 02, 2012 at 10:34:17PM +0800, Shawn Guo wrote:
On Wed, May 02, 2012 at 06:34:41PM +0800, Richard Zhao wrote:
Shawn, will you add clk ssi_ipg?
Ok, will do after Sascha's clk series gets settled on arm-soc tree.
I have a branch based on Mikes clk-next as of today, I will send it tomorrow.
Sascha
Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/mach-imx/clk-imx6q.c | 3 +++ arch/arm/mach-imx/mach-imx6q.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 9a03dcc..4ea0de0 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog"); clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi"); + clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL); + clk_register_clkdev(clk[ahb], "ahb", NULL); + clk_register_clkdev(clk[cko1], "cko1", NULL);
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) { c = clk_get_sys(clks_init_on[i], NULL); diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index d25c5d8..e9b2522 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -10,6 +10,8 @@ * http://www.gnu.org/copyleft/gpl.html */
+#include <linux/clkdev.h> +#include <linux/clk.h> #include <linux/delay.h> #include <linux/init.h> #include <linux/io.h> @@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev) return 0; }
+static void __init imx6q_cko1_setup(void) +{ + struct clk *cko1_sel, *ahb, *cko1; + unsigned long rate; + + cko1_sel = clk_get_sys(NULL, "cko1_sel"); + ahb = clk_get_sys(NULL, "ahb"); + cko1 = clk_get_sys(NULL, "cko1"); + if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) { + printk(KERN_ERR "cko1 setup failed!\n"); + goto put_clk; + } + clk_set_parent(cko1_sel, ahb); + rate = clk_round_rate(cko1, 16000000); + clk_set_rate(cko1, rate); + clk_register_clkdev(cko1, NULL, "0-000a"); +put_clk: + if (!IS_ERR(cko1_sel)) + clk_put(cko1_sel); + if (!IS_ERR(ahb)) + clk_put(ahb); + if (!IS_ERR(cko1)) + clk_put(cko1); +} + static void __init imx6q_sabrelite_init(void) { phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, ksz9021rn_phy_fixup); + imx6q_cko1_setup(); }
static void __init imx6q_init_machine(void)
On Fri, Apr 27, 2012 at 03:03:04PM +0800, Richard Zhao wrote:
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 3 +++ arch/arm/mach-imx/mach-imx6q.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 9a03dcc..4ea0de0 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog"); clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
clk_register_clkdev(clk[ahb], "ahb", NULL);
clk_register_clkdev(clk[cko1], "cko1", NULL);
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) { c = clk_get_sys(clks_init_on[i], NULL);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index d25c5d8..e9b2522 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -10,6 +10,8 @@
*/
+#include <linux/clkdev.h> +#include <linux/clk.h>
Nit: I would have <linux/clk.h> put before <linux/clkdev.h>.
#include <linux/delay.h> #include <linux/init.h> #include <linux/io.h> @@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev) return 0; }
+static void __init imx6q_cko1_setup(void)
I would say this is a sabrelite specific setup so far.
+{
- struct clk *cko1_sel, *ahb, *cko1;
- unsigned long rate;
- cko1_sel = clk_get_sys(NULL, "cko1_sel");
- ahb = clk_get_sys(NULL, "ahb");
- cko1 = clk_get_sys(NULL, "cko1");
- if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) {
printk(KERN_ERR "cko1 setup failed!\n");
pr_err
goto put_clk;- }
- clk_set_parent(cko1_sel, ahb);
- rate = clk_round_rate(cko1, 16000000);
- clk_set_rate(cko1, rate);
- clk_register_clkdev(cko1, NULL, "0-000a");
This dev_id looks a little strange to me. I understand that's the consequence of having sgtl5000 clock managed in ASoC machine driver imx-sgtl5000. IMO, having sgtl5000 driver manages its clock and looking up the clock with dev_id simply as "sgtl5000" makes more sense to me. Will put more comment on your imx-sgtl5000 clock patch.
Regards, Shawn
+put_clk:
- if (!IS_ERR(cko1_sel))
clk_put(cko1_sel);- if (!IS_ERR(ahb))
clk_put(ahb);- if (!IS_ERR(cko1))
clk_put(cko1);+}
static void __init imx6q_sabrelite_init(void) { phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, ksz9021rn_phy_fixup);
- imx6q_cko1_setup();
}
static void __init imx6q_init_machine(void)
1.7.5.4
On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
- clk_register_clkdev(cko1, NULL, "0-000a");
This dev_id looks a little strange to me. I understand that's the consequence of having sgtl5000 clock managed in ASoC machine driver imx-sgtl5000.
Just realised it's not the case. But shouldn't the name look like "sgtl5000.0-000a"?
On Tue, May 1, 2012 at 8:39 PM, Shawn Guo shawn.guo@linaro.org wrote:
On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
- clk_register_clkdev(cko1, NULL, "0-000a");
This dev_id looks a little strange to me. I understand that's the consequence of having sgtl5000 clock managed in ASoC machine driver imx-sgtl5000.
Just realised it's not the case. But shouldn't the name look like "sgtl5000.0-000a"?
Since this is common code, we should not hardcode the I2C bus as the codec can be connected in a I2C bus different of 0.
On Tue, May 01, 2012 at 11:47:55PM -0300, Fabio Estevam wrote:
On Tue, May 1, 2012 at 8:39 PM, Shawn Guo shawn.guo@linaro.org wrote:
On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
- clk_register_clkdev(cko1, NULL, "0-000a");
This dev_id looks a little strange to me. I understand that's the consequence of having sgtl5000 clock managed in ASoC machine driver imx-sgtl5000.
Just realised it's not the case. But shouldn't the name look like "sgtl5000.0-000a"?
Since this is common code, we should not hardcode the I2C bus as the codec can be connected in a I2C bus different of 0.
No, it's not. It's a sabrelite specific lookup which is done in imx6q_sabrelite_init, while imx6q_cko1_setup needs a better naming, as what I already commented.
In the end, this type of board specific clock lookup should really go into device tree.
On Tue, May 01, 2012 at 08:47:10PM +0800, Shawn Guo wrote:
On Fri, Apr 27, 2012 at 03:03:04PM +0800, Richard Zhao wrote:
Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/mach-imx/clk-imx6q.c | 3 +++ arch/arm/mach-imx/mach-imx6q.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index 9a03dcc..4ea0de0 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -419,6 +419,9 @@ int __init mx6q_clocks_init(void) clk_register_clkdev(clk[dummy], NULL, "20bc000.wdog"); clk_register_clkdev(clk[dummy], NULL, "20c0000.wdog"); clk_register_clkdev(clk[ssi1], NULL, "2028000.ssi");
clk_register_clkdev(clk[cko1_sel], "cko1_sel", NULL);
clk_register_clkdev(clk[ahb], "ahb", NULL);
clk_register_clkdev(clk[cko1], "cko1", NULL);
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++) { c = clk_get_sys(clks_init_on[i], NULL);
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index d25c5d8..e9b2522 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -10,6 +10,8 @@
*/
+#include <linux/clkdev.h> +#include <linux/clk.h>
Nit: I would have <linux/clk.h> put before <linux/clkdev.h>.
ok
#include <linux/delay.h> #include <linux/init.h> #include <linux/io.h> @@ -75,10 +77,36 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev) return 0; }
+static void __init imx6q_cko1_setup(void)
I would say this is a sabrelite specific setup so far.
ok
+{
- struct clk *cko1_sel, *ahb, *cko1;
- unsigned long rate;
- cko1_sel = clk_get_sys(NULL, "cko1_sel");
- ahb = clk_get_sys(NULL, "ahb");
- cko1 = clk_get_sys(NULL, "cko1");
- if (IS_ERR(cko1_sel) || IS_ERR(ahb) || IS_ERR(cko1)) {
printk(KERN_ERR "cko1 setup failed!\n");pr_err
ok
Thanks Richard
goto put_clk;- }
- clk_set_parent(cko1_sel, ahb);
- rate = clk_round_rate(cko1, 16000000);
- clk_set_rate(cko1, rate);
- clk_register_clkdev(cko1, NULL, "0-000a");
This dev_id looks a little strange to me. I understand that's the consequence of having sgtl5000 clock managed in ASoC machine driver imx-sgtl5000. IMO, having sgtl5000 driver manages its clock and looking up the clock with dev_id simply as "sgtl5000" makes more sense to me. Will put more comment on your imx-sgtl5000 clock patch.
Regards, Shawn
+put_clk:
- if (!IS_ERR(cko1_sel))
clk_put(cko1_sel);- if (!IS_ERR(ahb))
clk_put(ahb);- if (!IS_ERR(cko1))
clk_put(cko1);+}
static void __init imx6q_sabrelite_init(void) { phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK, ksz9021rn_phy_fixup);
- imx6q_cko1_setup();
}
static void __init imx6q_init_machine(void)
1.7.5.4
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com --- arch/arm/boot/dts/imx6q-sabrelite.dts | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index cdae2dd..20aa767 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -97,4 +97,17 @@ regulator-always-on; }; }; + + sound { + compatible = "fsl,imx6q-sabrelite-sgtl5000", + "fsl,imx-audio-sgtl5000"; + model = "imx6q-sabrelite-sgtl5000"; + ssi-controller = <&ssi1>; + audio-codec = <&codec>; + audio-routing = + "MIC_IN", "Mic Jack", + "Headphone Jack", "HP_OUT"; + mux-int-port = <1>; + mux-ext-port = <4>; + }; };
On Fri, Apr 27, 2012 at 03:03:05PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/boot/dts/imx6q-sabrelite.dts | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index cdae2dd..20aa767 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -97,4 +97,17 @@ regulator-always-on; }; };
- sound {
compatible = "fsl,imx6q-sabrelite-sgtl5000","fsl,imx-audio-sgtl5000";model = "imx6q-sabrelite-sgtl5000";ssi-controller = <&ssi1>;audio-codec = <&codec>;audio-routing ="MIC_IN", "Mic Jack",
Just curious if you have capture working without enabling Mic Bias power supplier.
Regards, Shawn
"Headphone Jack", "HP_OUT";mux-int-port = <1>;mux-ext-port = <4>;- };
};
1.7.5.4
On Tue, May 01, 2012 at 08:55:14PM +0800, Shawn Guo wrote:
On Fri, Apr 27, 2012 at 03:03:05PM +0800, Richard Zhao wrote:
From: Richard Zhao richard.zhao@linaro.org
Signed-off-by: Richard Zhao richard.zhao@linaro.org Signed-off-by: Richard Zhao richard.zhao@freescale.com
arch/arm/boot/dts/imx6q-sabrelite.dts | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts index cdae2dd..20aa767 100644 --- a/arch/arm/boot/dts/imx6q-sabrelite.dts +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts @@ -97,4 +97,17 @@ regulator-always-on; }; };
- sound {
compatible = "fsl,imx6q-sabrelite-sgtl5000","fsl,imx-audio-sgtl5000";model = "imx6q-sabrelite-sgtl5000";ssi-controller = <&ssi1>;audio-codec = <&codec>;audio-routing ="MIC_IN", "Mic Jack",Just curious if you have capture working without enabling Mic Bias power supplier.
I should add a "Mic Jack", "Mic Bias". When arecord, mic_bias_event is called. It's strange that it can not record any voice. debugging ...
Thanks Richard
Regards, Shawn
"Headphone Jack", "HP_OUT";mux-int-port = <1>;mux-ext-port = <4>;- };
};
1.7.5.4
participants (12)
-
Dong Aisheng -
Fabio Estevam -
Huang Shijie -
Laxman Dewangan -
Lothar Waßmann -
Mark Brown -
Richard Zhao -
Richard Zhao -
Russell King - ARM Linux -
Sascha Hauer -
Shawn Guo -
Vinod Koul