Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

Hi,
I've found the following problems in linux-3.5-rc5
(6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:

At boot:

------------[ cut here ]------------
WARNING: at drivers/clk/clk.c:508 __clk_enable+0x28/0x94()
Modules linked in:
[<c0018c7c>] (unwind_backtrace+0x0/0xf0) from [<c0020fc8>]
(warn_slowpath_common+0x4c/0x64)
[<c0020fc8>] (warn_slowpath_common+0x4c/0x64) from [<c0020ff8>]
(warn_slowpath_null+0x18/0x1c)
[<c0020ff8>] (warn_slowpath_null+0x18/0x1c) from [<c020a14c>]
(__clk_enable+0x28/0x94)
[<c020a14c>] (__clk_enable+0x28/0x94) from [<c020a1dc>] (clk_enable+0x24/0x58)
[<c020a1dc>] (clk_enable+0x24/0x58) from [<c0227f44>] (imx_ssi_probe+0x9c/0x374)
[<c0227f44>] (imx_ssi_probe+0x9c/0x374) from [<c018d018>]
(platform_drv_probe+0x14/0x18)
[<c018d018>] (platform_drv_probe+0x14/0x18) from [<c018bf44>]
(driver_probe_device+0xb0/0x1c4)
[<c018bf44>] (driver_probe_device+0xb0/0x1c4) from [<c018c0b8>]
(__driver_attach+0x60/0x84)
[<c018c0b8>] (__driver_attach+0x60/0x84) from [<c018a930>]
(bus_for_each_dev+0x48/0x84)
[<c018a930>] (bus_for_each_dev+0x48/0x84) from [<c018b044>]
(bus_add_driver+0x9c/0x214)
[<c018b044>] (bus_add_driver+0x9c/0x214) from [<c018c5c8>]
(driver_register+0xa0/0x12c)
[<c018c5c8>] (driver_register+0xa0/0x12c) from [<c00087d8>]
(do_one_initcall+0x94/0x16c)
[<c00087d8>] (do_one_initcall+0x94/0x16c) from [<c03a1280>]
(kernel_init+0xd4/0x19c)
[<c03a1280>] (kernel_init+0xd4/0x19c) from [<c0014940>]
(kernel_thread_exit+0x0/0x8)
---[ end trace 1b7118a416fcba5f ]---


When closing audio file:

visstrim login: Internal error: Oops: 17 [#1] PREEMPT ARM
Modules linked in:
CPU: 0    Tainted: G        W     (3.4.0-rc4-00070-gea01d31 #20)
PC is at snd_dmaengine_pcm_get_data+0x8/0x10
LR is at snd_imx_close+0xc/0x28
pc : [<c021d860>]    lr : [<c02288cc>]    psr: a0000013
sp : c3365ef8  ip : 00000000  fp : b6e67538
r10: c316ec00  r9 : c316aca0  r8 : c31706c0
r7 : c316fda0  r6 : c316abf0  r5 : c31706c0  r4 : c316abfc
r3 : 00000000  r2 : 00000001  r1 : c33400c0  r0 : c31706c0
Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 0005317f  Table: a3374000  DAC: 00000015
Process linphonec (pid: 443, stack limit = 0xc3364270)
Stack: (0xc3365ef8 to 0xc3366000)
5ee0:                                                       c316abfc c316fec0
5f00: c316abf0 c02269d0 c31706c0 c33400c0 c316e8e0 c327ba00 c31706c0 c30171d0
5f20: c33400c8 c021673c c316e800 c33400c0 c316e8e0 c02167b4 c33400c0 c24a6998
5f40: c24a87b8 00000000 00000008 c0089a3c 00000000 00000000 c3365f90 c33400c0
5f60: 00000000 c3023140 00000014 c0013b48 c3364000 00000000 b6e67538 c0086a4c
5f80: c3023140 c33400c0 00000001 c0086b1c 011b81c8 00000000 011b81c8 00000006
5fa0: c0013b48 c00139a0 011b81c8 00000000 00000014 011b81c8 b6db4000 b6d54bdc
5fc0: 011b81c8 00000000 011b81c8 00000006 b6e67548 00000670 011a9b08 b6e67538
5fe0: 011b8178 bebb5a58 b6d54bf0 b5e2813c 60000010 00000014 00000000 00000000
[<c021d860>] (snd_dmaengine_pcm_get_data+0x8/0x10) from [<c02288cc>]
(snd_imx_close+0xc/0x28)
[<c02288cc>] (snd_imx_close+0xc/0x28) from [<c02269d0>]
(soc_pcm_close+0x134/0x1c8)
[<c02269d0>] (soc_pcm_close+0x134/0x1c8) from [<c021673c>]
(snd_pcm_release_substream+0x54/0xa4)
[<c021673c>] (snd_pcm_release_substream+0x54/0xa4) from [<c02167b4>]
(snd_pcm_release+0x28/0x6c)
[<c02167b4>] (snd_pcm_release+0x28/0x6c) from [<c0089a3c>] (fput+0x108/0x23c)
[<c0089a3c>] (fput+0x108/0x23c) from [<c0086a4c>] (filp_close+0x6c/0x78)
[<c0086a4c>] (filp_close+0x6c/0x78) from [<c0086b1c>] (sys_close+0xc4/0x120)
[<c0086b1c>] (sys_close+0xc4/0x120) from [<c00139a0>]
(ret_fast_syscall+0x0/0x2c)
Code: e5831008 e12fff1e e59030a0 e59330bc (e5930008)


Moreover, the following 'clk_get()' fails in mx2_camera.c driver:

	pcdev->clk_emma = clk_get(NULL, "emma");
	if (IS_ERR(pcdev->clk_emma)) {
		err = PTR_ERR(pcdev->clk_emma);
		goto exit_free_irq;
	}


I know you guys have been doing some hard deep job with the clocks in
the i.MX. I've tried a bisect but it's highly thedious since there are
a lot of commits that do not compile:

git bisect start
# bad: [6e1c39c6b00d9141a82c231ba7c5e5b1716974b2] ALSA: hda - Fix
power-map regression for HP dv6 & co
git bisect bad 6e1c39c6b00d9141a82c231ba7c5e5b1716974b2
# good: [a23dc694828e3de96bf18e20459ba885ba91cb29] ASoC: imx: merge
sound/soc/imx into sound/soc/fsl
git bisect good a23dc694828e3de96bf18e20459ba885ba91cb29
# good: [94b5aff4c6f72fee6b0f49d49e4fa8b204e8ded9] Merge tag
'tty-3.5-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty
git bisect good 94b5aff4c6f72fee6b0f49d49e4fa8b204e8ded9
# good: [f2fde3a65e88330017b816faf2ef75f141d21375] Merge branch
'drm-core-next' of git://people.freedesktop.org/~airlied/linux
git bisect good f2fde3a65e88330017b816faf2ef75f141d21375
# bad: [858ee3784e8105467f1f3017f4ece51cb51d4830] ipc/mqueue: switch
back to using non-max values on create
git bisect bad 858ee3784e8105467f1f3017f4ece51cb51d4830
# bad: [27953437059c64d14086196eb96f43c78caa9db3] Merge tag 'clock' of
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect bad 27953437059c64d14086196eb96f43c78caa9db3
# good: [ece78b7df734726e790dcab207f463401ff80440] Merge branch
'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
git bisect good ece78b7df734726e790dcab207f463401ff80440
# good: [0877aa3908aaeeae8fc2850691668c4315d3db56] Merge tag
'defconfig' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect good 0877aa3908aaeeae8fc2850691668c4315d3db56
# good: [0877aa3908aaeeae8fc2850691668c4315d3db56] Merge tag
'defconfig' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc
git bisect good 0877aa3908aaeeae8fc2850691668c4315d3db56
# bad: [fcd8d84a585f3578a9ebdd27e757495a27415322] Merge branches
'spear/clock' and 'imx/clock' into next/clock
git bisect bad fcd8d84a585f3578a9ebdd27e757495a27415322
# good: [66a2886d867343eff6bf2646bea2c923d0cbf620] Merge branch
'spear/dt' into spear/clock
git bisect good 66a2886d867343eff6bf2646bea2c923d0cbf620
# skip: [b75c015143a4a6021731ff3e36718896381be94f] ARM i.MX: Add
common clock support for 2bit gate
git bisect skip b75c015143a4a6021731ff3e36718896381be94f
# skip: [a10bd67f1905b394f5a9bd610dfc8b9b9befac0e] ARM: imx: add
common clock support for pfd
git bisect skip a10bd67f1905b394f5a9bd610dfc8b9b9befac0e
# skip: [a3f6b9dbf2a964b85f0523e577807d6f63d6b29d] ARM: imx: add
common clock support for pllv3
git bisect skip a3f6b9dbf2a964b85f0523e577807d6f63d6b29d
# skip: [4f5a9fd341e8ffd825ecf56155df6fe6c3d732b1] Merge branch
'imx/pinctrl' into imx/clock
git bisect skip 4f5a9fd341e8ffd825ecf56155df6fe6c3d732b1
# skip: [6bbaec5676e4f475b0d78743cbd4c70a8804ce14] ARM i.MX25:
implement clocks using common clock framework
git bisect skip 6bbaec5676e4f475b0d78743cbd4c70a8804ce14
# skip: [4e7b6c9a6b4700cf121a0d5924f193db83cbd008] watchdog imx2:
prepare clk before enabling it
git bisect skip 4e7b6c9a6b4700cf121a0d5924f193db83cbd008
# skip: [32af7a830fe1625fa93900606f82c541f3b3de94] ARM: imx: add
common clock support for clk busy
git bisect skip 32af7a830fe1625fa93900606f82c541f3b3de94
# skip: [fdf7748b9f8d392a086560616bf112f0ba0c1f71] media mx3 camera:
prepare clk before enabling it
git bisect skip fdf7748b9f8d392a086560616bf112f0ba0c1f71
# skip: [a547b816a879b235ae0f90ae32d74effbb20d6d1] ARM i.MX: Add
common clock support for pllv2
git bisect skip a547b816a879b235ae0f90ae32d74effbb20d6d1
# skip: [c818f97bc3266f0fbf619f2348d951272f8ac335] ARM i.MX: remove
now unused clock files
git bisect skip c818f97bc3266f0fbf619f2348d951272f8ac335
# skip: [2af9e6db14db9a7a0a6510227352fb2e69f9d032] ARM i.MX: Add
common clock support for pllv1
git bisect skip 2af9e6db14db9a7a0a6510227352fb2e69f9d032
# skip: [2acd1b6f889c04f8f303a5a11993f60fda6a7885] ARM: i.MX6:
implement clocks using common clock framework
git bisect skip 2acd1b6f889c04f8f303a5a11993f60fda6a7885
# good: [7560e3f3581ed415828d3f431b8622fa38c2d133] dmaengine i.MX
SDMA: do not depend on grouped clocks
git bisect good 7560e3f3581ed415828d3f431b8622fa38c2d133
# skip: [4ec8c7f52654cdbb13770d04dc76f9a4615cd4e1] rtc: imx dryice:
Add missing clk_prepare
git bisect skip 4ec8c7f52654cdbb13770d04dc76f9a4615cd4e1
# good: [23b5e15a2994fb0c1444f92b76f09a482f32843c] clk: mxs: add mxs
specific clocks
git bisect good 23b5e15a2994fb0c1444f92b76f09a482f32843c
# skip: [5b48a6145466f1e2b58b31b1673ec413dabdab2a] ARM i.MX35:
implement clocks using common clock framework
git bisect skip 5b48a6145466f1e2b58b31b1673ec413dabdab2a
...
...
...

Do you have any hints of what could have been broken? I'll keep on
searching meanwhile.

Regards.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

The tag 'imx-clk-common-fixes' of repository
'git://git.pengutronix.de/git/imx/linux-2.6.git' is broken too:

  CC      arch/arm/mach-imx/cpu-imx27.o
  CC      arch/arm/mach-imx/pm-imx27.o
  CC      arch/arm/mach-imx/clk-imx27.o
In file included from arch/arm/mach-imx/clk-imx27.c:11:
arch/arm/mach-imx/clk.h: In function 'imx_clk_mux':
arch/arm/mach-imx/clk.h:73: warning: passing argument 3 of
'clk_register_mux' from incompatible pointer type
include/linux/clk-provider.h:255: note: expected 'char **' but
argument is of type 'const char **'
arch/arm/mach-imx/clk.h: In function 'imx_clk_fixed_factor':
arch/arm/mach-imx/clk.h:79: error: implicit declaration of function
'clk_register_fixed_factor'
arch/arm/mach-imx/clk.h:80: warning: return makes pointer from integer
without a cast
arch/arm/mach-imx/clk-imx27.c: In function 'mx27_clocks_init':
arch/arm/mach-imx/clk-imx27.c:189: error: implicit declaration of
function 'clk_register_clkdev'
make[1]: *** [arch/arm/mach-imx/clk-imx27.o] Error 1
make: *** [arch/arm/mach-imx] Error 2



-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

Hi Javier,

On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
<javier.martin@vista-silicon.com> wrote:
> Hi,
> I've found the following problems in linux-3.5-rc5
> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:

Could you please try the two attached patches?

They should fix the dma and mx2-camera issues.

I will try to look at the ssi problem, but my mx27pdk board still does
not have audio supported.

Regards,

Fabio Estevam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-clkdma.patch
Type: application/octet-stream
Size: 2637 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120703/b546b6ff/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-clkmx2cam.patch
Type: application/octet-stream
Size: 3983 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120703/b546b6ff/attachment-0003.obj>

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Tue, Jul 3, 2012 at 10:13 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Javier,
>
> On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
> <javier.martin@vista-silicon.com> wrote:
>> Hi,
>> I've found the following problems in linux-3.5-rc5
>> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
>
> Could you please try the two attached patches?
>
> They should fix the dma and mx2-camera issues.
>
> I will try to look at the ssi problem, but my mx27pdk board still does
> not have audio supported.

Just added audio support to mx27pdk and I could play audio
successfully with the patches I sent you previously.

I will submit them through the proper mailing lists.

Regards,

Fabio Estevam

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Sascha Hauer]

On Tue, Jul 03, 2012 at 01:09:52PM +0200, javier Martin wrote:
> Hi,
> I've found the following problems in linux-3.5-rc5
> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
> 
> At boot:
> 
> Moreover, the following 'clk_get()' fails in mx2_camera.c driver:
> 
> 	pcdev->clk_emma = clk_get(NULL, "emma");
> 	if (IS_ERR(pcdev->clk_emma)) {
> 		err = PTR_ERR(pcdev->clk_emma);
> 		goto exit_free_irq;
> 	}
> 
> 
> I know you guys have been doing some hard deep job with the clocks in
> the i.MX. I've tried a bisect but it's highly thedious since there are
> a lot of commits that do not compile:
> 

[...]

> Do you have any hints of what could have been broken? I'll keep on
> searching meanwhile.

Mainly these two problems exist:

- missing clk_prepare (or clk_prepare_enable)
- missing (or mismatching) clk_register_clkdev

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 |

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

Hi,

On 3 July 2012 15:13, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Javier,
>
> On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
> <javier.martin@vista-silicon.com> wrote:
>> Hi,
>> I've found the following problems in linux-3.5-rc5
>> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
>
> Could you please try the two attached patches?
>
> They should fix the dma and mx2-camera issues.
>
> I will try to look at the ssi problem, but my mx27pdk board still does
> not have audio supported.
>
> Regards,
>

thanks for the patches. The first one solves the issue with the audio.
Now I can do an aplay properly too.

However, second patch for the mx2-camera seems that it's not right
yet. I get a timeout here:

static int mx27_camera_emma_prp_reset(struct mx2_camera_dev *pcdev)
{
	u32 cntl;
	int count = 0;

	cntl = readl(pcdev->base_emma + PRP_CNTL);
	writel(PRP_CNTL_SWRST, pcdev->base_emma + PRP_CNTL);
	while (count++ < 100) {
		if (!(readl(pcdev->base_emma + PRP_CNTL) & PRP_CNTL_SWRST))
			return 0;
		barrier();
		udelay(1);
	}

	return -ETIMEDOUT;
}

Keeping this in mind I infer that probably emma clocks have not been
properly enabled.


-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

On 4 July 2012 09:48, javier Martin <javier.martin@vista-silicon.com> wrote:
> Hi,
>
> On 3 July 2012 15:13, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi Javier,
>>
>> On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
>> <javier.martin@vista-silicon.com> wrote:
>>> Hi,
>>> I've found the following problems in linux-3.5-rc5
>>> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
>>
>> Could you please try the two attached patches?
>>
>> They should fix the dma and mx2-camera issues.
>>
>> I will try to look at the ssi problem, but my mx27pdk board still does
>> not have audio supported.
>>
>> Regards,
>>
>
> thanks for the patches. The first one solves the issue with the audio.
> Now I can do an aplay properly too.
>
> However, second patch for the mx2-camera seems that it's not right
> yet. I get a timeout here:
>
> static int mx27_camera_emma_prp_reset(struct mx2_camera_dev *pcdev)
> {
>         u32 cntl;
>         int count = 0;
>
>         cntl = readl(pcdev->base_emma + PRP_CNTL);
>         writel(PRP_CNTL_SWRST, pcdev->base_emma + PRP_CNTL);
>         while (count++ < 100) {
>                 if (!(readl(pcdev->base_emma + PRP_CNTL) & PRP_CNTL_SWRST))
>                         return 0;
>                 barrier();
>                 udelay(1);
>         }
>
>         return -ETIMEDOUT;
> }
>
> Keeping this in mind I infer that probably emma clocks have not been
> properly enabled.
>

The problem is that in 'clk-imx27.c' the clocks for the emma are
defined as follows:

	clk_register_clkdev(clk[emma_ahb_gate], "ahb", "imx-emma");
	clk_register_clkdev(clk[emma_ipg_gate], "ipg", "imx-emma");

However, in the patch you submitted they are requested as shown:

	pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "ipg");
	if (IS_ERR(pcdev->clk_emma_ipg)) {
		err = PTR_ERR(pcdev->clk_emma_ipg);
		goto exit_kfree;
	}

	pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "ahb");
	if (IS_ERR(pcdev->clk_emma_ahb)) {
		err = PTR_ERR(pcdev->clk_emma_ahb);
		goto exit_kfree;
	}

Which means the dev_id used is "mx2-camera.0" instead of "imx-emma".
We cannot change the name in 'clk-imx27.c' to "mx2-camera.0" since
there is another device "mx2_emmaprp.c" which uses these clocks,
unless two different names can be associated with the same clock.

The other option, which is dirty but works, is using the following to
request clocks:

	pcdev->clk_emma_ipg = clk_get_sys("imx-emma", "ipg");
	if (IS_ERR(pcdev->clk_emma_ipg)) {
		err = PTR_ERR(pcdev->clk_emma_ipg);
		goto exit_kfree;
	}

	pcdev->clk_emma_ahb = clk_get_sys("imx-emma", "ahb");
	if (IS_ERR(pcdev->clk_emma_ahb)) {
		err = PTR_ERR(pcdev->clk_emma_ahb);
		goto exit_kfree;
	}

What do you think?

Regards.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Sascha Hauer]

On Wed, Jul 04, 2012 at 10:30:40AM +0200, javier Martin wrote:
> On 4 July 2012 09:48, javier Martin <javier.martin@vista-silicon.com> wrote:
> > Hi,
> >
> > On 3 July 2012 15:13, Fabio Estevam <festevam@gmail.com> wrote:
> >> Hi Javier,
> >>
> >> On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
> >> <javier.martin@vista-silicon.com> wrote:
> >>> Hi,
> >>> I've found the following problems in linux-3.5-rc5
> >>> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
> >>
> >> Could you please try the two attached patches?
> >>
> >> They should fix the dma and mx2-camera issues.
> >>
> >> I will try to look at the ssi problem, but my mx27pdk board still does
> >> not have audio supported.
> >>
> >> Regards,
> >>
> >
> > thanks for the patches. The first one solves the issue with the audio.
> > Now I can do an aplay properly too.
> >
> > However, second patch for the mx2-camera seems that it's not right
> > yet. I get a timeout here:
> >
> > static int mx27_camera_emma_prp_reset(struct mx2_camera_dev *pcdev)
> > {
> >         u32 cntl;
> >         int count = 0;
> >
> >         cntl = readl(pcdev->base_emma + PRP_CNTL);
> >         writel(PRP_CNTL_SWRST, pcdev->base_emma + PRP_CNTL);
> >         while (count++ < 100) {
> >                 if (!(readl(pcdev->base_emma + PRP_CNTL) & PRP_CNTL_SWRST))
> >                         return 0;
> >                 barrier();
> >                 udelay(1);
> >         }
> >
> >         return -ETIMEDOUT;
> > }
> >
> > Keeping this in mind I infer that probably emma clocks have not been
> > properly enabled.
> >
> 
> The problem is that in 'clk-imx27.c' the clocks for the emma are
> defined as follows:
> 
> 	clk_register_clkdev(clk[emma_ahb_gate], "ahb", "imx-emma");
> 	clk_register_clkdev(clk[emma_ipg_gate], "ipg", "imx-emma");
> 
> However, in the patch you submitted they are requested as shown:
> 
> 	pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "ipg");
> 	if (IS_ERR(pcdev->clk_emma_ipg)) {
> 		err = PTR_ERR(pcdev->clk_emma_ipg);
> 		goto exit_kfree;
> 	}
> 
> 	pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "ahb");
> 	if (IS_ERR(pcdev->clk_emma_ahb)) {
> 		err = PTR_ERR(pcdev->clk_emma_ahb);
> 		goto exit_kfree;
> 	}
> 
> Which means the dev_id used is "mx2-camera.0" instead of "imx-emma".
> We cannot change the name in 'clk-imx27.c' to "mx2-camera.0" since
> there is another device "mx2_emmaprp.c" which uses these clocks,
> unless two different names can be associated with the same clock.

This can be done just by calling clk_register_clkdev again with the
second name and this is also the preferred way of associating the same
clock to different devices.

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 |

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

On 4 July 2012 10:35, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Jul 04, 2012 at 10:30:40AM +0200, javier Martin wrote:
>> On 4 July 2012 09:48, javier Martin <javier.martin@vista-silicon.com> wrote:
>> > Hi,
>> >
>> > On 3 July 2012 15:13, Fabio Estevam <festevam@gmail.com> wrote:
>> >> Hi Javier,
>> >>
>> >> On Tue, Jul 3, 2012 at 8:09 AM, javier Martin
>> >> <javier.martin@vista-silicon.com> wrote:
>> >>> Hi,
>> >>> I've found the following problems in linux-3.5-rc5
>> >>> (6887a4131da3adaab011613776d865f4bcfb5678) in our Visstrim M10 board:
>> >>
>> >> Could you please try the two attached patches?
>> >>
>> >> They should fix the dma and mx2-camera issues.
>> >>
>> >> I will try to look at the ssi problem, but my mx27pdk board still does
>> >> not have audio supported.
>> >>
>> >> Regards,
>> >>
>> >
>> > thanks for the patches. The first one solves the issue with the audio.
>> > Now I can do an aplay properly too.
>> >
>> > However, second patch for the mx2-camera seems that it's not right
>> > yet. I get a timeout here:
>> >
>> > static int mx27_camera_emma_prp_reset(struct mx2_camera_dev *pcdev)
>> > {
>> >         u32 cntl;
>> >         int count = 0;
>> >
>> >         cntl = readl(pcdev->base_emma + PRP_CNTL);
>> >         writel(PRP_CNTL_SWRST, pcdev->base_emma + PRP_CNTL);
>> >         while (count++ < 100) {
>> >                 if (!(readl(pcdev->base_emma + PRP_CNTL) & PRP_CNTL_SWRST))
>> >                         return 0;
>> >                 barrier();
>> >                 udelay(1);
>> >         }
>> >
>> >         return -ETIMEDOUT;
>> > }
>> >
>> > Keeping this in mind I infer that probably emma clocks have not been
>> > properly enabled.
>> >
>>
>> The problem is that in 'clk-imx27.c' the clocks for the emma are
>> defined as follows:
>>
>>       clk_register_clkdev(clk[emma_ahb_gate], "ahb", "imx-emma");
>>       clk_register_clkdev(clk[emma_ipg_gate], "ipg", "imx-emma");
>>
>> However, in the patch you submitted they are requested as shown:
>>
>>       pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "ipg");
>>       if (IS_ERR(pcdev->clk_emma_ipg)) {
>>               err = PTR_ERR(pcdev->clk_emma_ipg);
>>               goto exit_kfree;
>>       }
>>
>>       pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "ahb");
>>       if (IS_ERR(pcdev->clk_emma_ahb)) {
>>               err = PTR_ERR(pcdev->clk_emma_ahb);
>>               goto exit_kfree;
>>       }
>>
>> Which means the dev_id used is "mx2-camera.0" instead of "imx-emma".
>> We cannot change the name in 'clk-imx27.c' to "mx2-camera.0" since
>> there is another device "mx2_emmaprp.c" which uses these clocks,
>> unless two different names can be associated with the same clock.
>
> This can be done just by calling clk_register_clkdev again with the
> second name and this is also the preferred way of associating the same
> clock to different devices.
>
> Sascha
>

So, we would have:

	clk_register_clkdev(clk[csi_ahb_gate], NULL, "mx2-camera.0");
	...
        clk_register_clkdev(clk[emma_ahb_gate], "ahb", "mx2-camera.0");
	clk_register_clkdev(clk[emma_ipg_gate], "ipg", "mx2-camera.0");
	clk_register_clkdev(clk[emma_ahb_gate], "ahb", "m2m-emmaprp.0");
	clk_register_clkdev(clk[emma_ipg_gate], "ipg", "m2m-emmaprp.0"));

And in mx2_camera.c:

	pcdev->clk_csi = devm_clk_get(&pdev->dev, NULL);
	if (IS_ERR(pcdev->clk_csi)) {
		dev_err(&pdev->dev, "Could not get csi clock\n");
		err = PTR_ERR(pcdev->clk_csi);
		goto exit_kfree;
	}

        pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "ipg");
        if (IS_ERR(pcdev->clk_emma_ipg)) {
                err = PTR_ERR(pcdev->clk_emma_ipg);
                goto exit_kfree;
        }

        pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "ahb");
        if (IS_ERR(pcdev->clk_emma_ahb)) {
                err = PTR_ERR(pcdev->clk_emma_ahb);
                goto exit_kfree;
        }

In this case, can we guarantee that the framework is able to tell the
difference between "csi_ahb_gate" and "emma_ahb_gate",
"emma_ipg_gate"? For instance, for requesting 'clk_csi' we use:

devm_clk_get(&pdev->dev, NULL);

since 'NULL' is handled as a wildcard, how can we make sure that we
are not requesting 'emma_ahb_gate' or 'emma_ipg_gate' instead?

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Wed, Jul 4, 2012 at 4:48 AM, javier Martin
<javier.martin@vista-silicon.com> wrote:

> thanks for the patches. The first one solves the issue with the audio.
> Now I can do an aplay properly too.

Ok, great.

> However, second patch for the mx2-camera seems that it's not right
> yet. I get a timeout here:

I see the same here too now. When I sent you the patches yesterday I
did not have access to the board.

Looking at the mx2_camera.c driver: wouldn't it make more sense to
handle the emma resources (clocks, irqs, etc) outside of this driver?
What about doing it in mx2_emmaprp.c instead?

Regards,

Fabio Estevam

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

On 4 July 2012 13:26, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Jul 4, 2012 at 4:48 AM, javier Martin
> <javier.martin@vista-silicon.com> wrote:
>
>> thanks for the patches. The first one solves the issue with the audio.
>> Now I can do an aplay properly too.
>
> Ok, great.
>
>> However, second patch for the mx2-camera seems that it's not right
>> yet. I get a timeout here:
>
> I see the same here too now. When I sent you the patches yesterday I
> did not have access to the board.
>
> Looking at the mx2_camera.c driver: wouldn't it make more sense to
> handle the emma resources (clocks, irqs, etc) outside of this driver?
> What about doing it in mx2_emmaprp.c instead?

mx2_emmaprp.c is a different driver for the same device. Its a mem2mem
driver that gets frames from memory in one video format and then
outputs frames in another video format. It hasn't got anything to do
with the mx2_camera.c apart from the fact they can't be used at the
same time since they share emma resources.

Regards.

-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Wed, Jul 4, 2012 at 8:30 AM, javier Martin
<javier.martin@vista-silicon.com> wrote:

> mx2_emmaprp.c is a different driver for the same device. Its a mem2mem
> driver that gets frames from memory in one video format and then
> outputs frames in another video format. It hasn't got anything to do
> with the mx2_camera.c apart from the fact they can't be used at the
> same time since they share emma resources.

Ok, understood.

So the original mx2_camera patch i sent you plus the clk-imx27.c
changes (calling clk_register_clkdev again with the
second name) fixes the issue for you?

If so, then maybe I can send the original mx2_camera patch to the
linux-media list.

Please confirm.

I will be able to test it later today.

Regards,

Fabio Estevam

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Wed, Jul 4, 2012 at 8:41 AM, Fabio Estevam <festevam@gmail.com> wrote:

> So the original mx2_camera patch i sent you plus the clk-imx27.c
> changes (calling clk_register_clkdev again with the
> second name) fixes the issue for you?

Ok, I have tested the mx2_camera patch I sent you plus the one below:

--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -250,8 +250,10 @@ int __init mx27_clocks_init(unsigned long fref)
        clk_register_clkdev(clk[i2c2_ipg_gate], NULL, "imx-i2c.1");
        clk_register_clkdev(clk[owire_ipg_gate], NULL, "mxc_w1.0");
        clk_register_clkdev(clk[kpp_ipg_gate], NULL, "imx-keypad");
-       clk_register_clkdev(clk[emma_ahb_gate], "ahb", "imx-emma");
-       clk_register_clkdev(clk[emma_ipg_gate], "ipg", "imx-emma");
+       clk_register_clkdev(clk[emma_ahb_gate], "ahb", "mx2-camera.0");
+       clk_register_clkdev(clk[emma_ipg_gate], "ipg", "mx2-camera.0");
+       clk_register_clkdev(clk[emma_ahb_gate], "ahb", "m2m-emmaprp.0");
+       clk_register_clkdev(clk[emma_ipg_gate], "ipg", "m2m-emmaprp.0");
        clk_register_clkdev(clk[iim_ipg_gate], "iim", NULL);
        clk_register_clkdev(clk[gpio_ipg_gate], "gpio", NULL);
        clk_register_clkdev(clk[brom_ahb_gate], "brom", NULL);

,and I get the following on probe:

soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
mx2-camera mx2-camera.0: Camera driver attached to camera 0
ov2640 0-0030: Product ID error fb:fb
mx2-camera mx2-camera.0: Camera driver detached from camera 0
mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 665000

Looks like an i2c error when trying to read the ov2640 ID.

Is your camera connected to i2c? Do you get it probed correctly?

Any suggestions are welcome.

Thanks,

Fabio Estevam

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Wed, Jul 4, 2012 at 3:23 PM, Fabio Estevam <festevam@gmail.com> wrote:

> ,and I get the following on probe:
>
> soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
> mx2-camera mx2-camera.0: Camera driver attached to camera 0
> ov2640 0-0030: Product ID error fb:fb
> mx2-camera mx2-camera.0: Camera driver detached from camera 0
> mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 665000
>
> Looks like an i2c error when trying to read the ov2640 ID.

Just for comparison, here is the log from a 3.4.4 kernel:

soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
mx2-camera mx2-camera.0: Camera driver attached to camera 0
ov2640 0-0030: ov2640 Product ID 26:42 Manufacturer ID 7f:a2
i2c i2c-0: OV2640 Probed
mx2-camera mx2-camera.0: Camera driver detached from camera 0
mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 443333
Driver for 1-wire Dallas network protocol

i2detetc also sees 0x30 (camera address) on 3.4.4:

root at freescale /$ i2cdetect 0
WARNING! This program can confuse your I2C bus, cause data loss and worse!
I will probe file /dev/i2c-0.
I will probe address range 0x03-0x77.
Continue? [Y/n] Y
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:          -- -- -- -- -- -- -- -- -- -- -- -- --
10: 10 -- -- -- -- -- -- -- -- -- -- -- -- 1d -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- 76 --

,but on 3.5-rc5 (or linux-next) the camera address is not detected via
i2cdetect.

Wolfram,

Have you seen any issues lately with mx27 i2c?

Regards,

Fabio Estevam

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

The attached patch solves mx2_camera clock problems.

It can be also found by the following subject in the lists:

media: i.MX27: Fix emma-prp clocks in mx2_camera.c


-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mx2_camera.patch
Type: application/octet-stream
Size: 5129 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120706/400b297d/attachment.obj>

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[Fabio Estevam]

On Fri, Jul 6, 2012 at 3:44 AM, javier Martin
<javier.martin@vista-silicon.com> wrote:
> The attached patch solves mx2_camera clock problems.
>
> It can be also found by the following subject in the lists:
>
> media: i.MX27: Fix emma-prp clocks in mx2_camera.c

Just tried your patch against today's linux-next and I still get the
following on a mx27pdk:

soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
mx2-camera mx2-camera.0: Camera driver attached to camera 0
ov2640 0-0030: Product ID error fb:fb
mx2-camera mx2-camera.0: Camera driver detached from camera 0
mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 665000

Are you able to probe i2c correctly on your board?

Clock problems in i.MX27 Visstrim M10 (linux-3.5-rc5).

[javier Martin]

On 6 July 2012 14:32, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Jul 6, 2012 at 3:44 AM, javier Martin
> <javier.martin@vista-silicon.com> wrote:
>> The attached patch solves mx2_camera clock problems.
>>
>> It can be also found by the following subject in the lists:
>>
>> media: i.MX27: Fix emma-prp clocks in mx2_camera.c
>
> Just tried your patch against today's linux-next and I still get the
> following on a mx27pdk:
>
> soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0
> mx2-camera mx2-camera.0: Camera driver attached to camera 0
> ov2640 0-0030: Product ID error fb:fb
> mx2-camera mx2-camera.0: Camera driver detached from camera 0
> mx2-camera mx2-camera.0: MX2 Camera (CSI) driver probed, clock frequency: 665000
>
> Are you able to probe i2c correctly on your board?

Yes, I am and my patch does not touch any of the I2C settings or clocks.


-- 
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com