Re: [PATCH v5 4/5] iommu/omap: adapt to runtime pm
On Tue, Nov 20, 2012 at 2:05 AM, Omar Ramirez Luna omar.l...@linaro.org wrote: @@ -1022,7 +1019,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev) release_mem_region(res-start, resource_size(res)); iounmap(obj-regbase); - clk_put(obj-clk); + pm_runtime_disable(obj-dev); + dev_info(pdev-dev, %s removed\n, obj-name); kfree(obj); return 0; I still believe this is not needed. The device framework does that when removing the device, and does it more properly, with __pm_runtime_disable(dev, false). Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Beagleboard xM - play video file : BUG: scheduling while atomic: queue1:src/92/0x0000008e , then kernel panic
On Thu, Oct 25, 2012 at 12:49 AM, Omar Ramirez Luna omar.rami...@copitl.com wrote: On Wed, Oct 24, 2012 at 10:34 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Selso, hopefully you don't mind, but I'll forward this to the linux-omap mailing list, as this seems to be an interesting kernel problem in tidspbridge. Omar, any ideas? I don't see tidspbridge trace paths (but I don't discard something wrong either ;)), however I do see a framebuffer, dss, dispc trace, might be a good point to start checking: Oops, you are right. CONFIG_TIDSPBRIDGE_CACHE_LINE_CHECK=y I wasn't aware that now gst-dsp supported this, might be time to update mine. Yes, it's supported now. Took me a lot of changes to manage to do it, but it's there. Anyway, right now I have lots of debugging enabled and specifically the one that spits scheduling while atomic with kernel 3.7-rc2, and I'm not seeing this with the fakesink decode, and the encoder to a file, I don't have framebuffer nor display though. It's probably a framebuffer problem =/ -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Beagleboard xM - play video file : BUG: scheduling while atomic: queue1:src/92/0x0000008e , then kernel panic
] (__schedule+0x60/0x798) eline to PLAYING[ 467.480987] [c05c2d1c] (__schedule+0x60/0x798) from [c05c183c] (schedule_timeout+0x1dc/0x218) ... New clock:[ 467.500213] [c05c183c] (schedule_timeout+0x1dc/0x218) from [c05c2a34] (wait_for_common+0x104/0x1bc) [ 467.520050] [c05c2a34] (wait_for_common+0x104/0x1bc) from [c0362f00] (omap_dispc_wait_for_irq_interruptible_timeout+0x4c/0x84) [ 467.542510] [c0362f00] (omap_dispc_wait_for_irq_interruptible_timeout+0x4c/0x84) from [c0364158] (dss_mgr_wait_for_vsync+0x50/0x60) [ 467.564208] [c0364158] (dss_mgr_wait_for_vsync+0x50/0x60) from [c03773fc] (omapfb_ioctl+0x9cc/0xed0) [ 467.583099] [c03773fc] (omapfb_ioctl+0x9cc/0xed0) from [c0345e9c] (do_fb_ioctl+0x56c/0x5a8) [ 467.601196] [c0345e9c] (do_fb_ioctl+0x56c/0x5a8) from [c011ffa4] (vfs_ioctl+0x24/0x40) [ 467.618804] [c011ffa4] (vfs_ioctl+0x24/0x40) from [c0120ab4] (do_vfs_ioctl+0x560/0x5a8) [ 467.636535] [c0120ab4] (do_vfs_ioctl+0x560/0x5a8) from [c0120b48] (sys_ioctl+0x4c/0x6c) [ 467.654205] [c0120b48] (sys_ioctl+0x4c/0x6c) from [c000d4c0] (ret_fast_syscall+0x0/0x30) [ 467.672943] BUG: scheduling while atomic: queue0:src/91/0x011b [ 467.688415] Modules linked in: tidspbridge(C) mailbox_mach mailbox [ 467.704132] [c001369c] (unwind_backtrace+0x0/0xe0) from [c05b99e4] (__schedule_bug+0x48/0x5c) [ 467.722412] [c05b99e4] (__schedule_bug+0x48/0x5c) from [c05c2d1c] (__schedule+0x60/0x798) [ 467.740325] [c05c2d1c] (__schedule+0x60/0x798) from [c007d67c] (futex_wait_queue_me+0xf8/0x114) [ 467.758819] [c007d67c] (futex_wait_queue_me+0xf8/0x114) from [c007d7cc] (futex_wait+0xd4/0x210) [ 467.777252] [c007d7cc] (futex_wait+0xd4/0x210) from [c007f67c] (do_futex+0xc0/0xab4) [ 467.794647] [c007f67c] (do_futex+0xc0/0xab4) from [c0080194] (sys_futex+0x124/0x168) [ 467.812042] [c0080194] (sys_futex+0x124/0x168) from [c000d4c0] (ret_fast_syscall+0x0/0x30) [ 467.830200] Unable to handle kernel paging request at virtual address b380da00 [ 467.846893] pgd = dc44c000 [ 467.858581] [b380da00] *pgd=9c457831, *pte=9f88c307, *ppte=9f88ca37 [ 467.874267] Internal error: Oops: 81f [#1] SMP ARM [ 467.887908] Modules linked in: tidspbridge(C) mailbox_mach mailbox [ 467.903106] CPU: 0Tainted: GWC(3.6.0+ #1) [ 467.917388] PC is at 0xb6b7ec50 [ 467.929077] LR is at 0xb66f21e8 [ 467.940643] pc : [b6b7ec50]lr : [b66f21e8]psr: 2010 [ 467.940643] sp : b4125ca8 ip : b36670e0 fp : 80808080 [ 467.969177] r10: 80808080 r9 : 80808080 r8 : 80808080 [ 467.982696] r7 : 80808080 r6 : 80808080 r5 : 80808080 r4 : 80808080 [ 467.997558] r3 : 80808080 r2 : 0008c9c0 r1 : b36670a0 r0 : b380da00 [ 468.012237] Flags: nzCv IRQs on FIQs on Mode USER_32 ISA ARM Segment user [ 468.027648] Control: 10c5387d Table: 9c44c019 DAC: 0015 [ 468.041473] Process dspvdec0:src (pid: 94, stack limit = 0xdc4742f8) [ 468.056091] ---[ end trace 7346e43bee93ae33 ]--- [ 468.068725] note: dspvdec0:src[94] exited with preempt_count 141 [ 468.084838] NOHZ: local_softirq_pending 40 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Beagleboard xM - play video file : BUG: scheduling while atomic: queue1:src/92/0x0000008e , then kernel panic
On Wed, Oct 24, 2012 at 5:49 PM, Selso Liberado selso.liber...@gmail.com wrote: I wished that someone agreed to mail him about this. Is the mailing list your talking about is located with this url ? http://linux.omap.com/mailman/listinfo Yes, they are in CC now. I did also work with the linux-omap tree repo, and may repeat the test with this one. It should work with a vanilla kernel. I think you might have better luck with older kernels, 3.2, or maybe even 3.0. I don't recall the last one I tried, but it was working fine. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP3/4: iommu: adapt to runtime pm
On Thu, Oct 18, 2012 at 1:51 AM, Omar Ramirez Luna omar.l...@linaro.org wrote: On 15 October 2012 22:23, Felipe Contreras felipe.contre...@gmail.com wrote: On probe this patch does pm_runtime_enable, however this doesn't mean the device is turned ON or resumed or kept ON all the time. In fact it's the other way around; pm_runtime_enable turns off the power (if it's ON). pm_runtime_enable just decrements the disable_depth counter. Doesn't turn off anything by itself. pm_runtime_enable turns on power management, without it the device remains in whatever state it booted. device_del device_pm_remove pm_runtime_remove __pm_runtime_disable - HERE I'm not entirely convinced _iommu_ follows this path, it doesn't create any devices nor register them... whereas mailbox does create devices and mailbox does follow this path for the devices created which are child devices. So this path won't take care of the omap-iommu device pm_runtime_disable. Are you sure? What code-path calls omap_iommu_remove then? Anyway, it was just an observation. I've seen other code do this. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP3/4: iommu: adapt to runtime pm
On Fri, Oct 12, 2012 at 3:06 AM, Omar Ramirez Luna omar.l...@linaro.org wrote:
Use runtime PM functionality interfaced with hwmod enable/idle
functions, to replace direct clock operations and sysconfig
handling.
Dues to reset sequence, pm_runtime_put_sync must be used, to avoid
possible operations with the module under reset.
I already made most of these comments, but here they go again.
@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
}
}
- clk_enable(obj-clk);
+ pm_runtime_get_sync(obj-dev);
err = arch_iommu-enable(obj);
- clk_disable(obj-clk);
The device will never go to sleep, until iommu_disable is called.
clk_enable - pm_runtime_get_sync, clk_disable pm_runtime_put.
@@ -288,7 +285,7 @@ static int load_iotlb_entry(struct omap_iommu *obj,
struct iotlb_entry *e)
if (!obj || !obj-nr_tlb_entries || !e)
return -EINVAL;
- clk_enable(obj-clk);
+ pm_runtime_get_sync(obj-dev);
iotlb_lock_get(obj, l);
if (l.base == obj-nr_tlb_entries) {
@@ -318,7 +315,7 @@ static int load_iotlb_entry(struct omap_iommu *obj,
struct iotlb_entry *e)
cr = iotlb_alloc_cr(obj, e);
if (IS_ERR(cr)) {
- clk_disable(obj-clk);
+ pm_runtime_put_sync(obj-dev);
return PTR_ERR(cr);
}
If I'm correct, the above pm_runtime_get/put are redundant, because
the count can't possibly reach 0 because of the reason I explained
before.
The same for all the cases below.
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct
platform_device *pdev)
release_mem_region(res-start, resource_size(res));
iounmap(obj-regbase);
- clk_put(obj-clk);
+ pm_runtime_disable(obj-dev);
This will turn on the device unnecessarily, wasting power, and there's
no need for that, kfree will take care of that without resuming.
dev_info(pdev-dev, %s removed\n, obj-name);
kfree(obj);
return 0;
Also, I still think that something like this is needed:
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -,8 +,17 @@ static struct clk cam_mclk = {
.recalc = followparent_recalc,
};
+static struct clk cam_fck = {
+ .name = cam_fck,
+ .ops= clkops_omap2_iclk_dflt,
+ .parent = l3_ick,
+ .enable_reg = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),
+ .enable_bit = OMAP3430_EN_CAM_SHIFT,
+ .clkdm_name = cam_clkdm,
+ .recalc = followparent_recalc,
+};
+
static struct clk cam_ick = {
- /* Handles both L3 and L4 clocks */
.name = cam_ick,
.ops= clkops_omap2_iclk_dflt,
.parent = l4_ick,
@@ -3394,6 +3403,7 @@ static struct omap_clk omap3xxx_clks[] = {
CLK(omapdss_dss, ick, dss_ick_3430es2,
CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
CLK(NULL, dss_ick, dss_ick_3430es2,
CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
CLK(NULL, cam_mclk, cam_mclk, CK_34XX | CK_36XX),
+ CLK(NULL, cam_fck, cam_fck, CK_34XX | CK_36XX),
CLK(NULL, cam_ick, cam_ick, CK_34XX | CK_36XX),
CLK(NULL, csi2_96m_fck, csi2_96m_fck, CK_34XX | CK_36XX),
CLK(NULL, usbhost_120m_fck, usbhost_120m_fck,
CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: OMAP3/4: iommu: adapt to runtime pm
On Fri, Oct 12, 2012 at 7:46 PM, Omar Ramirez Luna omar.l...@linaro.org wrote:
On 12 October 2012 02:48, Felipe Contreras felipe.contre...@gmail.com wrote:
I already made most of these comments, but here they go again.
I replied to all, but here it goes again:
Mostly, but not all :)
@@ -142,11 +142,10 @@ static int iommu_enable(struct omap_iommu *obj)
}
}
- clk_enable(obj-clk);
+ pm_runtime_get_sync(obj-dev);
err = arch_iommu-enable(obj);
- clk_disable(obj-clk);
The device will never go to sleep, until iommu_disable is called.
clk_enable - pm_runtime_get_sync, clk_disable pm_runtime_put.
Which is what you want... why would you want your iommu to be disabled
if the client of that iommu could request a translation?
That's the whole point of *dynamic* pm; _when_ the client wants to
request a translation, _then_ the device is waken up, which is what I
believe the code currently does.
After your patch, even if I don't use the camera, or the DSP, the
iommu devices will be enabled, and will be consuming energy *all the
time*. Which I don't think is what we want.
I'm not saying I have a solution, I'm simply saying that's what's
going to happen if I'm correct.
Remember that these iommus, sit along of other processors not on the
main processor side. So, this code should enable it for the other
processor to use, and there is no point where the processor can say
I'm not using it, shut it down or I'm using it, turn it on in the
middle of execution, other than suspend/resume and if supported,
autosuspend.
I understand, but perhaps there should be?
@@ -288,7 +285,7 @@ static int load_iotlb_entry(struct omap_iommu *obj,
struct iotlb_entry *e)
if (!obj || !obj-nr_tlb_entries || !e)
return -EINVAL;
- clk_enable(obj-clk);
+ pm_runtime_get_sync(obj-dev);
iotlb_lock_get(obj, l);
if (l.base == obj-nr_tlb_entries) {
@@ -318,7 +315,7 @@ static int load_iotlb_entry(struct omap_iommu *obj,
struct iotlb_entry *e)
cr = iotlb_alloc_cr(obj, e);
if (IS_ERR(cr)) {
- clk_disable(obj-clk);
+ pm_runtime_put_sync(obj-dev);
return PTR_ERR(cr);
}
If I'm correct, the above pm_runtime_get/put are redundant, because
the count can't possibly reach 0 because of the reason I explained
before.
The same for all the cases below.
You can access this paths through debugfs, some of them doesn't work
if the module is not enabled first, but in future if you just want to
idle the iommu withouth freeing, these are needed to debug.
BTW, the next patch in the series: ARM: OMAP: iommu: pm runtime save
and restore context, let's you do a pm_runtime_[enable|put] through
save/restore ctx functions, which is just for compatibility on how isp
code uses the save and restore code.
All right, it has been some time since I've touched pm code, so if you say so.
@@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct
platform_device *pdev)
release_mem_region(res-start, resource_size(res));
iounmap(obj-regbase);
- clk_put(obj-clk);
+ pm_runtime_disable(obj-dev);
This will turn on the device unnecessarily, wasting power, and there's
no need for that, kfree will take care of that without resuming.
Left aside the aesthetics of having balanced calls, the device will be
enabled if there was a pending resume to be executed, otherwise it
won't, kfree won't increment the disable_depth counter and I don't
think that freeing the pointer is enough reason to ignore
pm_runtime_disable.
You are doing __pm_runtime_disable(dev, true), kfree will do
__pm_runtime_disable(dev, false), which is what we want. Both will
decrement the disable_depth.
But at least you agree that there's a chance that the device will be waken up.
Also, I still think that something like this is needed:
...
+static struct clk cam_fck = {
+ .name = cam_fck,
+ .ops= clkops_omap2_iclk_dflt,
+ .parent = l3_ick,
+ .enable_reg = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_ICLKEN),
a cam_fck name to enable the ick?
Yeap, according to the TRM. Take a look at 12.3 Camera ISP Integration
Fig 12-50.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/15] tidspbridge: Fix VM_PFNMAP mapping
On Wed, Sep 19, 2012 at 2:06 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: VMAs marked with the VM_PFNMAP flag have no struct page associated with the memory PFNs. Don't call get_page()/put_page() on the pages supposedly associated with the PFNs. I don't see anything wrong with the patch, but I think there's a lot of changes, and perhaps a bit more of explanation of how it's doing this might help. Also, it seems to be reordering match_exact_map_obj(), which messes up the diff a bit. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] arm: omap: fix trivial warnings for dspbridge
arch/arm/plat-omap/devices.c: In function 'omap_dsp_reserve_sdram_memblock':
arch/arm/plat-omap/devices.c:170: warning: format '%x' expects type 'unsigned
int', but argument 3 has type 'phys_addr_t'
arch/arm/mach-omap2/dsp.c: In function 'omap_dsp_init':
arch/arm/mach-omap2/dsp.c:60: warning: format '%x' expects type 'unsigned int',
but argument 3 has type 'phys_addr_t'
arch/arm/mach-omap2/dsp.c:60: warning: format '%x' expects type 'unsigned int',
but argument 4 has type 'phys_addr_t'
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
arch/arm/mach-omap2/dsp.c|5 +++--
arch/arm/plat-omap/devices.c |4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/dsp.c b/arch/arm/mach-omap2/dsp.c
index 74f18f2..3376388 100644
--- a/arch/arm/mach-omap2/dsp.c
+++ b/arch/arm/mach-omap2/dsp.c
@@ -57,8 +57,9 @@ static int __init omap_dsp_init(void)
if (pdata-phys_mempool_base) {
pdata-phys_mempool_size = CONFIG_TIDSPBRIDGE_MEMPOOL_SIZE;
- pr_info(%s: %x bytes @ %x\n, __func__,
- pdata-phys_mempool_size, pdata-phys_mempool_base);
+ pr_info(%s: %llx bytes @ %llx\n, __func__,
+ (unsigned long long)pdata-phys_mempool_size,
+ (unsigned long long)pdata-phys_mempool_base);
}
pdev = platform_device_alloc(omap-dsp, -1);
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index 60278f4..09b07d2 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -167,8 +167,8 @@ void __init omap_dsp_reserve_sdram_memblock(void)
paddr = arm_memblock_steal(size, SZ_1M);
if (!paddr) {
- pr_err(%s: failed to reserve %x bytes\n,
- __func__, size);
+ pr_err(%s: failed to reserve %llx bytes\n,
+ __func__, (unsigned long long)size);
return;
}
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] staging: tidspbridge: fix build errors for OMAP2_L4_IO_ADDRESS
drivers/staging/tidspbridge/core/tiomap3430.c: In function 'bridge_brd_start': drivers/staging/tidspbridge/core/tiomap3430.c:425: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' drivers/staging/tidspbridge/core/wdt.c: In function 'dsp_wdt_init': drivers/staging/tidspbridge/core/wdt.c:56: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' The proper fix will come later. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/staging/tidspbridge/include/dspbridge/dbdefs.h |8 1 file changed, 8 insertions(+) diff --git a/drivers/staging/tidspbridge/include/dspbridge/dbdefs.h b/drivers/staging/tidspbridge/include/dspbridge/dbdefs.h index c8f4645..f9a0d16 100644 --- a/drivers/staging/tidspbridge/include/dspbridge/dbdefs.h +++ b/drivers/staging/tidspbridge/include/dspbridge/dbdefs.h @@ -28,6 +28,14 @@ #define PG_ALIGN_LOW(addr, pg_size) ((addr) PG_MASK(pg_size)) #define PG_ALIGN_HIGH(addr, pg_size) (((addr)+(pg_size)-1) PG_MASK(pg_size)) +/* + * NOTE: Please update to use ioremap + read/write + */ + +#define OMAP2_L4_IO_OFFSET 0xb200 +#define IOMEM(x) ((void __force __iomem *)(x)) +#define OMAP2_L4_IO_ADDRESS(pa)IOMEM((pa) + OMAP2_L4_IO_OFFSET) + /* API return value and calling convention */ #define DBAPI int -- 1.7.10 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: fix build errors for OMAP2_L4_IO_ADDRESS
On Mon, Apr 23, 2012 at 5:39 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Apr 23, 2012 at 05:34:14PM +0300, Felipe Contreras wrote: drivers/staging/tidspbridge/core/tiomap3430.c: In function 'bridge_brd_start': drivers/staging/tidspbridge/core/tiomap3430.c:425: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' drivers/staging/tidspbridge/core/wdt.c: In function 'dsp_wdt_init': drivers/staging/tidspbridge/core/wdt.c:56: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' The proper fix will come later. Hmm. Given that this is staging stuff, which hasn't been making much progress out of staging, I'd suggest not doing the quick fix on it, but spending the time to implement the proper fix now and move the code closer to the point where it can move out of staging. Omar is already on that. You might get a gold star from Greg for moving this forward... In my experience gold stars from Greg mean nothing. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Test: please ignore
Greg KH wrote: On Sun, Apr 22, 2012 at 10:57:17AM +0100, Russell King - ARM Linux wrote: I've noticed these failure from time to time in my omap4 randconfig builds: drivers/staging/tidspbridge/core/tiomap3430.c: In function 'bridge_brd_start': drivers/staging/tidspbridge/core/tiomap3430.c:425: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' drivers/staging/tidspbridge/core/wdt.c: In function 'dsp_wdt_init': drivers/staging/tidspbridge/core/wdt.c:56: error: implicit declaration of function 'OMAP2_L4_IO_ADDRESS' Looking at git, it looks like there is some activity with tidspbridge (by Víctor). Is there any interest in fixing these errors for v3.4 (or even v3.4-rc5)? Along those lines, what's the plans on getting this code _out_ of staging? I haven't seen anything going on here in a while... Test 6. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm: omap: fix trivial warnings for dspbridge
arch/arm/plat-omap/devices.c: In function 'omap_dsp_reserve_sdram_memblock':
arch/arm/plat-omap/devices.c:170: warning: format '%x' expects type 'unsigned
int', but argument 3 has type 'phys_addr_t'
arch/arm/mach-omap2/dsp.c: In function 'omap_dsp_init':
arch/arm/mach-omap2/dsp.c:60: warning: format '%x' expects type 'unsigned int',
but argument 3 has type 'phys_addr_t'
arch/arm/mach-omap2/dsp.c:60: warning: format '%x' expects type 'unsigned int',
but argument 4 has type 'phys_addr_t'
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
arch/arm/mach-omap2/dsp.c|5 +++--
arch/arm/plat-omap/devices.c |4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-omap2/dsp.c b/arch/arm/mach-omap2/dsp.c
index 74f18f2..7665c49 100644
--- a/arch/arm/mach-omap2/dsp.c
+++ b/arch/arm/mach-omap2/dsp.c
@@ -57,8 +57,9 @@ static int __init omap_dsp_init(void)
if (pdata-phys_mempool_base) {
pdata-phys_mempool_size = CONFIG_TIDSPBRIDGE_MEMPOOL_SIZE;
- pr_info(%s: %x bytes @ %x\n, __func__,
- pdata-phys_mempool_size, pdata-phys_mempool_base);
+ pr_info(%s: %llu bytes @ %llu\n, __func__,
+ (unsigned long long)pdata-phys_mempool_size,
+ (unsigned long long)pdata-phys_mempool_base);
}
pdev = platform_device_alloc(omap-dsp, -1);
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index 60278f4..ffccec7 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -167,8 +167,8 @@ void __init omap_dsp_reserve_sdram_memblock(void)
paddr = arm_memblock_steal(size, SZ_1M);
if (!paddr) {
- pr_err(%s: failed to reserve %x bytes\n,
- __func__, size);
+ pr_err(%s: failed to reserve %llu bytes\n,
+ __func__, (unsigned long long)size);
return;
}
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm: omap: fix trivial warnings for dspbridge
On Sat, Apr 21, 2012 at 2:34 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
--- a/arch/arm/mach-omap2/dsp.c
+++ b/arch/arm/mach-omap2/dsp.c
@@ -57,8 +57,9 @@ static int __init omap_dsp_init(void)
if (pdata-phys_mempool_base) {
pdata-phys_mempool_size = CONFIG_TIDSPBRIDGE_MEMPOOL_SIZE;
- pr_info(%s: %x bytes @ %x\n, __func__,
- pdata-phys_mempool_size, pdata-phys_mempool_base);
+ pr_info(%s: %llu bytes @ %llu\n, __func__,
No, don't change a unprefixed hex number to a decimal number. Keep
the same formatting, just fix the warning. Changing the base of the
displayed number when there's no hex prefix to it is just plain idiotic
and creates confusion. Think: is the number 12345678 output by one of
these a hex number or a decimal number?
Besides, base addresses _should_ be hex numbers. I'd agree that sizes
should probably be decimal, but as none of these locations you're fixing
had 0x prefixes, I'd strongly advise to leave them as-is - esp. for a
patch allegedly just fixing warnings.
Right, I was too fast copying what Documentation/printk-formats.txt
suggested (%llu). So s/%llu/%llx/? I prefer to keep the size also as
hex, as it's usually how it's configured.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
2012/2/15 Víctor M. Jáquez L. vjaq...@igalia.com: On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote: On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. Yes! please! and use Ohad's rproc thingy. I thought rproc is tied to elf for now. What would be the steps to unify that common format? I guess we will depend on TI for that... Do we? But this common format would be specific for tidspbridge, I don't think it makes sense for these images to have that constraint. Certainly rproc doesn't have it, and that one is not on staging. In any case, the proposed patch looks good. We can deal about these futuristic situations later on. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] staging: tidspbridge: detecting wdt from baseimage
On Wed, Feb 15, 2012 at 5:00 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: This is an alternate solution for: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq... In-band Error seen by IVA_SS at address 0 Instead of setting CONFIG_TIDSPBRIDGE_WDT3 to 'default y', these patches include the code to be compiled for runtime detection of this feature based on baseimage symbols. As mentioned in the description of the first patch, we will lose the option to set a current overflow timeout and the default will be kept as 5 seconds. This can be replaced by a debugfs/sysfs entry if needed, I didn't include such a patch as we first need to decide which fix to take (between this and default y for CONFIG_TIDSPBRIDGE_WDT3[1]). [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg62119.html LGTM. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP34xx
On Thu, Feb 9, 2012 at 2:47 AM, Greg KH gre...@linuxfoundation.org wrote: Show me an OMAP user that actually runs a mainline kernel :) I haven't used anything but mainline kernel since years when I test stuff on my Nokia N900. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Tue, Feb 14, 2012 at 3:06 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 11, 2012 3:03 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 10, 2012 12:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. I'm out on travel but will be able to look at it on Monday. Well, I think it is the right way, you look on the firmware if it has WDT you use it if it doesn't then you don't. Rather than guessing if you have the feature. It would be like reading a config option in the firmware. Yeah, but it's not really firmware, it's an operating system image. I can be running Linux there, and I might have implemented WDT. How is that code going to find that out? Tidspbridge loader doesn't support that use case, baseimage and let's say a dsp-linuximg won't have the same layout, hence it won't be able to parse and load the latter. When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. WDT could be detected by prepending common symbols into the baseimages or just making the new images to treat all peripherals as resources, that is, the new baseimg should actually request the wdt3 as any other clock. I see, but if wdt3 is requested as another clock, the Linux ARM side would still need to know how to threat the WDT. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: musb: fix pm_runtime mismatch
On Mon, Feb 13, 2012 at 2:54 PM, Grazvydas Ignotas nota...@gmail.com wrote: On Thu, Jan 12, 2012 at 5:40 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Jan 12, 2012 at 1:56 PM, Grazvydas Ignotas nota...@gmail.com wrote: On Mon, Dec 19, 2011 at 10:01 PM, Felipe Contreras felipe.contre...@nokia.com wrote: From: Felipe Contreras felipe.contre...@gmail.com In musb_init_controller() there's a pm_runtime_put(), but there's no pm_runtime_get(), which creates a mismatch that causes the driver to sleep when it shouldn't. This patch is causing problems here, the device never suspends, I've verified musb_runtime_suspend and omap2430_runtime_suspend are never called in my setup, unless I revert this patch. I'm using musb with ethernet gadget; cable removal also doesn't suspend musb. It seems pm_runtime_put() that was removed was balancing out pm_runtime_get in musb_platform_init(), which is called at the beginning of musb_init_controller(). I think this pm_runtime_get() from musb_platform_init() should be moved to musb_init_controller() to stop confusing people and pm_runtime_put() needs to be brought back. I can do a patch if nobody objects. You mean have both get and put in musb_init_controller()? That's OK, it would be balanced then, but you would have to remove the pm_runtime_put() from omap2430_remove() that presumably was balancing the pm_runtime_get_sync() in omap2430_musb_init(). This was introduced in 7acc619[1], but it wasn't triggered in my setup until 18a2689[2] was merged to Linus' branch at point df0914[3]. IOW; when PM is working as it was supposed to. However, it seems most of the time this is used in a way that keeps the counter above 0, so nobody noticed. Also, it seems to depend on the configuration used in versions before 3.1, but not later (or in it). I found the problem by loading isp1704_charger before any usb gadgets: http://article.gmane.org/gmane.linux.kernel/1226122 isp1704_charger seems to be doing otg_io_read(), and it's perfectly normal that musb can be suspended at the time isp1704 decides to do this. I think proper fix is to wake up must on musb_ulpi_read() and musb_ulpi_write() instead. I guess so. If you send these patches I can give them a try. Sent them a week ago, would be nice if you tried them. Sorry, I just started hacking with my N900 again. I will probably test them later today. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 10, 2012 12:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. I'm out on travel but will be able to look at it on Monday. Well, I think it is the right way, you look on the firmware if it has WDT you use it if it doesn't then you don't. Rather than guessing if you have the feature. It would be like reading a config option in the firmware. Yeah, but it's not really firmware, it's an operating system image. I can be running Linux there, and I might have implemented WDT. How is that code going to find that out? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 5:35 PM, Justin P. Mattock justinmatt...@gmail.com wrote: On Thu, 9 Feb 2012 21:18:49 -0800 Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... confused, greg k-h so there are _two_ issues that need to be fixed here: 1) the warning fix. 2) the whole default y thing that Mr. Torvalds is talking about. (the first fix(in my mind)would need to go first before anything else) Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to have to supply the proper fix for the warning(my C skills only take me so far!) below is my go at sending a _dependency_ fix for this, but could be totally wrong.. From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 7:18 AM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? The warning doesn't come from the driver, and AFAIK it's a valid warning. Now, I haven't explored the code, so I don't know what's going on, I just know Omar said this fixes the warning, and that there's no easy way around it. I fail to understand why this warning happens, I don't know either, I just know enabling TIDSPBRIDGE_WDT3 fixes it. why it depends on the firmware, Because it just does. Does your firmware have WDT? Yes = TIDSPBRIDGE_WDT3 on, No = TIDSPBRIDGE_WDT3 off. What is so complicated about this? and why you can't detect it at runtime to not do it. The DSP firmwmare is an operating system. You even run Linux on it. We just don't know if has WDT support or not. Omar is familiar with the code regarding the WDT, and he said it was not easy to detect if enabling WDT failed or not. And how it all ties into a kconfig option... Some people want it, some people don't. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 6:16 PM, Greg KH gre...@linuxfoundation.org wrote: On Fri, Feb 10, 2012 at 06:05:59PM +0200, Felipe Contreras wrote: From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. What firmware? What do you think is the purpose of tidspbridge? Why not document this properly somewhere in the help entries? It is documented (kinda). If you are using a non-standard baseimage, you should know what is the watchdog timer, and whether or not your baseimage supports it. Fortunately, the vast majority of users don't need to care about it. They can just use the default. Why not detect this automatically in the kernel based on the firmware version? There is no firmware version. The firmware, or rather baseimage, is an operating system. You can't detect if Linux has USB support based on the Linux version, can you? Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. So, for the typical firmware, you do want this on, so the patch makes sense. Yes. How about the code be fixed so that it doesn't generate this type of warning when using the typical firmware, instead of having to rely on confusing Kconfig entries. Patches welcome. I for one don't know a way, and I'd rather concentrate on one of the myriad other tasks that need to be done for this driver. I just thought it would be nice to make it easier for most users to avoid this warning. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 7:35 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. Then you want this option disabled. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. There's many configurations that are not needed for booting, yet they are 'default y'. No. There's a reason they are *configurations*, and there's a reason they are 'default y'. Again, no NEW config options should be default y unless you really have a good reason. I already gave you a good reason, and this came from Linus Torvalds. http://article.gmane.org/gmane.linux.kernel/995419 Notice how he agreed that setting defaults was a good idea. I can't point to the mails of other people that also agreed this was the direction to go. You keep saying no options should be 'default y', but you don't say where that policy comes from. And I'll take that a step further and say, if you want this to be default y, then why even make it a config option at all and not just build it in whenever the dependancy is detected? The dependency can't be detected. So, care to just send a patch removing this config option instead? No. That's not what anybody wants. Or, if bad things happen if the option is turned off, then fix those, don't paper over it by trying to enable the option, that's
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 8:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I already gave you a good reason, and this came from Linus Torvalds. http://article.gmane.org/gmane.linux.kernel/995419 Notice how he agreed that setting defaults was a good idea. I can't point to the mails of other people that also agreed this was the direction to go. Er, I can, if you want. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 8:59 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 08:41:55PM +0200, Felipe Contreras wrote: On Thu, Feb 9, 2012 at 7:35 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. Then you want this option disabled. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. There's many configurations that are not needed for booting, yet they are 'default y'. Again, for historical reasons, yes, but do you see that happening for new code these days? Yes, I already explained why, and I already pointed to a link where Linus says it's the right thing to do. Here it is again: http://article.gmane.org/gmane.linux.kernel/995419 It is needed to simplify defconfigs. And if it does get set to 'y' for new code, you have seen the yelling, right? I didn't parse that. Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have
Re: [PATCH 0/4] staging: tidspbridge: remove desgin-by-contract macros
2012/2/7 Víctor Manuel Jáquez Leal vjaq...@igalia.com: These patches removes all the design-by-contract (DBC) macros in the tidspbridge. Also it removes the config option CONFIG_TIDSPBRIDGE_DEBUG. The patches are applied above these submitted patches: http://thread.gmane.org/gmane.linux.kernel/1246081 The main reasons to remove these DBC macros are * The kernel in general doesn't follow the DBC approach * They only provide a needless verbosity, since they are available only when CONFIG_TIDSPBRIDGE_DEBUG is enabled. If they were useful they should be left for the dynamic debugging always. So, if they are not used, is dead code at the end, and should be removed. * Deleting them means less code to maintain As the Emperor Joseph II told to Mozart in the motion picture Amadeus: It's quality work. And there are simply too many notes, that's all. Just cut a few and it will be perfect. The whole series is fine by me :) Even the warnings that don't depend on DEBUG have been neglected anyway =/ Will send some patches for a few of them soon. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: patch tty: serial: OMAP: ensure FIFO levels are set correctly in non-DMA added to tty tree
On Fri, Feb 3, 2012 at 6:07 AM, NeilBrown ne...@suse.de wrote: If I then enabled runtime_pm with a 5 second autosuspend delay: CORE is still permanently ON (I think the USB might be doing that). Maybe related to this: http://thread.gmane.org/gmane.linux.usb.general/56096 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 5:22 AM, Justin P. Mattock justinmatt...@gmail.com wrote: not sure if I see this warning or not on my machine, but is there a fix for the warning? I would rather see that, than hide it with setting: default y (if that's what its doing!) No, there's no fix. IIRC Omar said it would not be easy. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: tidspbridge: fix incorrect free to drv_datap
On Tue, Jan 31, 2012 at 8:43 PM, Dan Carpenter dan.carpen...@oracle.com wrote: How often do people rmmod things on a production system? Hopefully, never right? That's right... At least in recent versions of the tidspbrdige, that have recovery support. Before, we needed a script to detect MMU faults, and reload the module =/ IIRC this is still the case on the Nokia N900. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] staging: tidspbridge: enable watchdog by default
From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y help WTD3 is managed by DSP and once it is enabled, DSP side bridge is in charge of refreshing the timer before overflow, if the DSP hangs MPU -- 1.7.9 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mfd: twl4030: trivial code-style fixes
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/mfd/twl4030-irq.c | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index b69bb51..b31f920 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -59,7 +59,6 @@
#define REG_PIH_ISR_P2 0x02
#define REG_PIH_SIR0x03/* for testing */
-
/* Linux could (eventually) use either IRQ line */
static int irq_line;
@@ -111,7 +110,8 @@ static int nr_sih_modules;
#define TWL4030_MODULE_INT_PWR TWL4030_MODULE_INT
-/* Order in this table matches order in PIH_ISR. That is,
+/*
+ * Order in this table matches order in PIH_ISR. That is,
* BIT(n) in PIH_ISR is sih_modules[n].
*/
/* sih_modules_twl4030 is used both in twl4030 and twl5030 */
@@ -309,6 +309,7 @@ static irqreturn_t handle_twl4030_pih(int irq, void *devid)
return IRQ_HANDLED;
}
+
/*--*/
/*
@@ -337,7 +338,6 @@ static int twl4030_init_sih_modules(unsigned line)
memset(buf, 0xff, sizeof buf);
sih = sih_modules;
for (i = 0; i nr_sih_modules; i++, sih++) {
-
/* skip USB -- it's funky */
if (!sih-bytes_ixr)
continue;
@@ -352,7 +352,8 @@ static int twl4030_init_sih_modules(unsigned line)
pr_err(twl4030: err %d initializing %s %s\n,
status, sih-name, IMR);
- /* Maybe disable exclusive mode; buffer second pending irq;
+ /*
+* Maybe disable exclusive mode; buffer second pending irq;
* set Clear-On-Read (COR) bit.
*
* NOTE that sometimes COR polarity is documented as being
@@ -382,7 +383,8 @@ static int twl4030_init_sih_modules(unsigned line)
if (sih-irq_lines = line)
continue;
- /* Clear pending interrupt status. Either the read was
+ /*
+* Clear pending interrupt status. Either the read was
* enough, or we need to write those bits. Repeat, in
* case an IRQ is pending (PENDDIS=0) ... that's not
* uncommon with PWR_INT.PWRON.
@@ -398,7 +400,8 @@ static int twl4030_init_sih_modules(unsigned line)
status = twl_i2c_write(sih-module, buf,
sih-mask[line].isr_offset,
sih-bytes_ixr);
- /* else COR=1 means read sufficed.
+ /*
+* else COR=1 means read sufficed.
* (for most SIH modules...)
*/
}
@@ -410,7 +413,8 @@ static int twl4030_init_sih_modules(unsigned line)
static inline void activate_irq(int irq)
{
#ifdef CONFIG_ARM
- /* ARM requires an extra step to clear IRQ_NOREQUEST, which it
+ /*
+* ARM requires an extra step to clear IRQ_NOREQUEST, which it
* sets on behalf of every irq_chip. Also sets IRQ_NOPROBE.
*/
set_irq_flags(irq, IRQF_VALID);
@@ -622,9 +626,7 @@ static irqreturn_t handle_twl4030_sih(int irq, void *data)
static unsigned twl4030_irq_next;
-/* returns the first IRQ used by this SIH bank,
- * or negative errno
- */
+/* returns the first IRQ used by this SIH bank, or negative errno */
int twl4030_sih_setup(int module)
{
int sih_mod;
@@ -688,7 +690,6 @@ int twl4030_sih_setup(int module)
/* FIXME need a call to reverse twl4030_sih_setup() ... */
-
/*--*/
/* FIXME pass in which interrupt line we'll use ... */
@@ -711,7 +712,8 @@ int twl4030_init_irq(int irq_num, unsigned irq_base,
unsigned irq_end)
twl4030_irq_base = irq_base;
- /* install an irq handler for each of the SIH modules;
+ /*
+* install an irq handler for each of the SIH modules;
* clone dummy irq_chip since PIH can't *do* anything
*/
twl4030_irq_chip = dummy_irq_chip;
--
1.7.9
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] power: isp1704: fix probe error path
We enable power, but don't disable it in case of an error. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/power/isp1704_charger.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c index b806667..cce6211 100644 --- a/drivers/power/isp1704_charger.c +++ b/drivers/power/isp1704_charger.c @@ -470,6 +470,7 @@ fail0: dev_err(pdev-dev, failed to register isp1704 with error %d\n, ret); + isp1704_charger_set_power(isp, 0); return ret; } -- 1.7.9 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? And using that logic, we should start removing all configurations in the kernel that have 'default y'. No. There's a reason they are *configurations*, and there's a reason they are 'default y'. You might have noticed Linus' rant about ARM defconfigs (LWN article[1]), and the solution to that was simplified configs (make savedefconfig), and the ideal would be to have no defconfigs at all. But in order to achieve that ideal goal (or at least move in that direction), the Kconfig files need to have good defaults--this was also agreed in the discussion. If you look at arch/arm/configs/, there are still quite a lot of configs there, and many not small at all. If Linus goes forward with his threat to remove ARM defconfigs, we would be in a bit of a situation, specially since there's still a lot of Kconfigs that have not been fixed. There's plenty of other reasons why 'default y' is good. This change is good, it helps everyone, and it doesn't hurt anyone. But if you like, I can put forward a case and take it with the penguin chief. [1] http://lwn.net/Articles/391372/ -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] staging: tidspbridge: fix incorrect free to drv_datap
On Wed, Feb 1, 2012 at 8:58 AM, Dan Carpenter dan.carpen...@oracle.com wrote: On Tue, Jan 31, 2012 at 09:39:00PM +0200, Felipe Contreras wrote: On Tue, Jan 31, 2012 at 8:43 PM, Dan Carpenter dan.carpen...@oracle.com wrote: How often do people rmmod things on a production system? Hopefully, never right? That's right... At least in recent versions of the tidspbrdige, that have recovery support. Before, we needed a script to detect MMU faults, and reload the module =/ IIRC this is still the case on the Nokia N900. If you have a script that's doing rmmods then that's different. I didn't know about that. *Had*, there's no need any more. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tidspbridge: Rename module from bridgedriver to tidspbridge
On Mon, Jan 30, 2012 at 7:34 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Tue, Jan 24, 2012 at 3:25 PM, Joe Perches j...@perches.com wrote: tidspbridge when built as a module is named bridgedriver. bridgedriver is not a particularly good module name. tidspbridge is what the source is named. That seems a more appropriate module name too as it describes the hardware function better. Signed-off-by: Joe Perches j...@perches.com --- drivers/staging/tidspbridge/Makefile | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tidspbridge/Makefile b/drivers/staging/tidspbridge/Makefile index fd6a276..8c8c92a 100644 --- a/drivers/staging/tidspbridge/Makefile +++ b/drivers/staging/tidspbridge/Makefile @@ -1,4 +1,4 @@ -obj-$(CONFIG_TIDSPBRIDGE) += bridgedriver.o +obj-$(CONFIG_TIDSPBRIDGE) += tidspbridge.o libgen = gen/gh.o gen/uuidutil.o libcore = core/chnl_sm.o core/msg_sm.o core/io_sm.o core/tiomap3430.o \ @@ -13,7 +13,7 @@ libdload = dynload/cload.o dynload/getsection.o dynload/reloc.o \ dynload/tramp.o libhw = hw/hw_mmu.o -bridgedriver-y := $(libgen) $(libservices) $(libcore) $(libpmgr) $(librmgr) \ +tidspbridge-y := $(libgen) $(libservices) $(libcore) $(libpmgr) $(librmgr) \ $(libdload) $(libhw) #Machine dependent Sound good to me, and unless there is an objection, it also gives plenty of time for the change to be noticed. Yeah, I agree. I also have thought on doing this. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Mon, Jan 16, 2012 at 11:24 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 2:37 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Mon, Jan 16, 2012 at 7:01 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 10:59 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE)
extern void omapdrm_reserve_vram(void);
#else
static inline void omapdrm_reserve_vram(void) { }
#endif
Like how it's done with DSP stuff.
right, but then you'd miss the omapdrm_reserve_vram() call at startup..
Why?
I guess drm.o is ending up in a module, but the code that calls that
(in common.c) is not in a module, so you get an unresolved symbol at
link
Right... But that code can be moved as well. Just like with the bridge.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] omap: mailbox: trivial whitespace fix
Signed-off-by: Felipe Contreras felipe.contre...@nokia.com
---
arch/arm/plat-omap/mailbox.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index ad80112..ad32621 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -307,7 +307,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
if (!--mbox-use_count) {
free_irq(mbox-irq, mbox);
tasklet_kill(mbox-txq-tasklet);
- flush_work_sync(mbox-rxq-work);
+ flush_work_sync(mbox-rxq-work);
mbox_queue_free(mbox-txq);
mbox_queue_free(mbox-rxq);
}
--
1.7.8
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] staging: tidspbridge: remove unused header
2012/1/23 Víctor Manuel Jáquez Leal vjaq...@igalia.com: No functional changes. The header file drv_interface.h was only used locally, hence there's no need to have it. Also the only prototyped functions were the file_operations callbacks, then this commit moves them up to avoid prototyping too. 'Also' is a keyword that usually denotes that we are talking about two patches here: one that reshuffles the code, and another one that removes the header. But looks good to me either way. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] staging: tidspbridge: more readable code
2012/1/23 Víctor Manuel Jáquez Leal vjaq...@igalia.com: Uppercase function names are not pretty. Also the code flow readability is enhanced. Looks good to me. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Tue, Jan 24, 2012 at 5:54 PM, Rob Clark rob.cl...@linaro.org wrote:
On Tue, Jan 24, 2012 at 9:33 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Mon, Jan 16, 2012 at 11:24 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 2:37 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Mon, Jan 16, 2012 at 7:01 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 10:59 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE)
extern void omapdrm_reserve_vram(void);
#else
static inline void omapdrm_reserve_vram(void) { }
#endif
Like how it's done with DSP stuff.
right, but then you'd miss the omapdrm_reserve_vram() call at startup..
Why?
I guess drm.o is ending up in a module, but the code that calls that
(in common.c) is not in a module, so you get an unresolved symbol at
link
Right... But that code can be moved as well. Just like with the bridge.
Hmm, looks like for dsp bridge the memory reservation is moved to
devices.c. Although I'm not entirely sure how that is better... and
there is precedent for both approaches, ie. omapfb works like omapdrm,
and tidspbridge works as you suggest.
seems a bit bikeshed'ish to me
I will send a patch to fix omapfb, perhaps after that it will be a bit
more clear how it should be done :) (if it gets accepted)
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Fri, Jan 13, 2012 at 11:19 PM, Rob Clark rob.cl...@linaro.org wrote: On Fri, Jan 13, 2012 at 2:59 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Jan 13, 2012 at 10:41 PM, Rob Clark rob.cl...@linaro.org wrote: diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile index 9a58461..b86e6cb 100644 --- a/arch/arm/plat-omap/Makefile +++ b/arch/arm/plat-omap/Makefile @@ -4,7 +4,7 @@ # Common support obj-y := common.o sram.o clock.o devices.o dma.o mux.o \ - usb.o fb.o counter_32k.o + usb.o fb.o counter_32k.o drm.o Should be something like this: obj-$(CONFIG_DRM_OMAP) += drm.o btw, how does that work if CONFIG_DRM_OMAP=m? Do you end up w/ a plat-omap module? Yes, and platform drivers are supposed to be loaded automatically at boot-time by udev (or similar). -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Mon, Jan 16, 2012 at 6:37 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 8:12 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Jan 13, 2012 at 11:19 PM, Rob Clark rob.cl...@linaro.org wrote:
On Fri, Jan 13, 2012 at 2:59 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Jan 13, 2012 at 10:41 PM, Rob Clark rob.cl...@linaro.org wrote:
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 9a58461..b86e6cb 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -4,7 +4,7 @@
# Common support
obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
- usb.o fb.o counter_32k.o
+ usb.o fb.o counter_32k.o drm.o
Should be something like this:
obj-$(CONFIG_DRM_OMAP) += drm.o
btw, how does that work if CONFIG_DRM_OMAP=m? Do you end up w/ a
plat-omap module?
Yes, and platform drivers are supposed to be loaded automatically at
boot-time by udev (or similar).
oh, but this won't work, because common.c has to call
omapdrm_reserve_vram() (similar to how omapfb_reserve_sdram_memblock()
works).. so I think it has to stay the way it is in this patch.
#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE)
extern void omapdrm_reserve_vram(void);
#else
static inline void omapdrm_reserve_vram(void) { }
#endif
Like how it's done with DSP stuff.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Mon, Jan 16, 2012 at 7:01 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 10:59 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Mon, Jan 16, 2012 at 6:37 PM, Rob Clark rob.cl...@linaro.org wrote:
On Mon, Jan 16, 2012 at 8:12 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Jan 13, 2012 at 11:19 PM, Rob Clark rob.cl...@linaro.org wrote:
On Fri, Jan 13, 2012 at 2:59 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Fri, Jan 13, 2012 at 10:41 PM, Rob Clark rob.cl...@linaro.org wrote:
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 9a58461..b86e6cb 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -4,7 +4,7 @@
# Common support
obj-y := common.o sram.o clock.o devices.o dma.o mux.o \
- usb.o fb.o counter_32k.o
+ usb.o fb.o counter_32k.o drm.o
Should be something like this:
obj-$(CONFIG_DRM_OMAP) += drm.o
btw, how does that work if CONFIG_DRM_OMAP=m? Do you end up w/ a
plat-omap module?
Yes, and platform drivers are supposed to be loaded automatically at
boot-time by udev (or similar).
oh, but this won't work, because common.c has to call
omapdrm_reserve_vram() (similar to how omapfb_reserve_sdram_memblock()
works).. so I think it has to stay the way it is in this patch.
#if defined(CONFIG_DRM_OMAP) || defined(CONFIG_DRM_OMAP_MODULE)
extern void omapdrm_reserve_vram(void);
#else
static inline void omapdrm_reserve_vram(void) { }
#endif
Like how it's done with DSP stuff.
right, but then you'd miss the omapdrm_reserve_vram() call at startup..
Why?
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap2+: add drm device
On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote: On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote: +/* files from staging that contain platform data structure definitions */ +#include ../../../drivers/staging/omapdrm/omap_priv.h +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h btw, I'm not a huge fan of doing #includes this way, so if someone has a better suggestion (given that staging drivers should not have headers outside of drivers/staging) please let me know Move those structs to /arch/arm/plat-omap/drm.h? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap2+: add drm device
On Fri, Jan 13, 2012 at 9:53 PM, Rob Clark r...@ti.com wrote: On Fri, Jan 13, 2012 at 1:49 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Jan 13, 2012 at 9:46 PM, Rob Clark r...@ti.com wrote: On Fri, Jan 13, 2012 at 1:41 PM, Rob Clark rob.cl...@linaro.org wrote: +/* files from staging that contain platform data structure definitions */ +#include ../../../drivers/staging/omapdrm/omap_priv.h +#include ../../../drivers/staging/omapdrm/omap_dmm_tiler.h btw, I'm not a huge fan of doing #includes this way, so if someone has a better suggestion (given that staging drivers should not have headers outside of drivers/staging) please let me know Move those structs to /arch/arm/plat-omap/drm.h? Can I do that? Maybe it depends of if you consider those structs as part of the driver, or part of the platform? Why not? The platform is using them in arch/arm/plat-omap/drm.c. I guess that looks like how it was handled for dspbridge, so maybe that means it is a suitable precedent.. Indeed, the way I see it is this: imagine there's another driver in staging (or maybe even out of tree), that requires this data. It would simply include plat-omap/drm.h to fill this. OK, this is pretty far-fetched, but demonstrates the point: platform data should be completely independent of the drivers. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap2+: add drm device
On Fri, Jan 13, 2012 at 9:41 PM, Rob Clark rob.cl...@linaro.org wrote:
+void omapdrm_reserve_vram(void)
+{
+#ifdef CONFIG_CMA
+ /* Create private 32MiB contiguous memory area for omapdrm.0 device
+ * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
+ * then the amount of memory we need goes up..
+ */
/*
* Foo.
*/
+ dma_declare_contiguous(omap_drm_device.dev, 32*SZ_1M, 0, 0);
32 * SZ1_M
+#else
+# warning CMA is not enabled, there may be limitations about scanout
buffer allocations on OMAP3 and earlier
+#endif
+}
+
+#endif
diff --git a/arch/arm/plat-omap/drm.h b/arch/arm/plat-omap/drm.h
new file mode 100644
index 000..56e0c0e
--- /dev/null
+++ b/arch/arm/plat-omap/drm.h
Maybe this should go to include/plat
+#ifndef __PLAT_OMAP_DRM_H__
+#define __PLAT_OMAP_DRM_H__
I see a lot of headers using this form:
__ARCH_OMAP_FOO_H
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] omap2+: add drm device
On Fri, Jan 13, 2012 at 10:41 PM, Rob Clark rob.cl...@linaro.org wrote: diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile index 9a58461..b86e6cb 100644 --- a/arch/arm/plat-omap/Makefile +++ b/arch/arm/plat-omap/Makefile @@ -4,7 +4,7 @@ # Common support obj-y := common.o sram.o clock.o devices.o dma.o mux.o \ - usb.o fb.o counter_32k.o + usb.o fb.o counter_32k.o drm.o Should be something like this: obj-$(CONFIG_DRM_OMAP) += drm.o No? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] OMAP: I2C: Fix the mismatch of pm_runtime enable and disable
On Wed, Jan 11, 2012 at 4:25 PM, Shubhrajyoti shubhrajy...@ti.com wrote: On Wednesday 11 January 2012 07:29 PM, Grazvydas Ignotas wrote: On Wed, Jan 11, 2012 at 3:21 PM, Shubhrajyoti D shubhrajy...@ti.com wrote: Currently the i2c driver calls the pm_runtime_enable and never the disable. This may cause a warning when pm_runtime_enable checks for the count match.Attempting to fix the same by calling pm_runtime_disable in the error and the remove path. I remember seeing Felipe doing the reverse to musb here: http://marc.info/?l=linux-omapm=132432610700952w=2 so I'm confused here. Strange however I see many drivers doing the same in the kernel on greping . Besides I expect a warn to come up. Felipe could you explain the issue? When do you see the warning? kfree(dev) should disable runtime pm, but without waking up the device. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: musb: fix pm_runtime mismatch
On Thu, Jan 12, 2012 at 1:56 PM, Grazvydas Ignotas nota...@gmail.com wrote: On Mon, Dec 19, 2011 at 10:01 PM, Felipe Contreras felipe.contre...@nokia.com wrote: From: Felipe Contreras felipe.contre...@gmail.com In musb_init_controller() there's a pm_runtime_put(), but there's no pm_runtime_get(), which creates a mismatch that causes the driver to sleep when it shouldn't. This patch is causing problems here, the device never suspends, I've verified musb_runtime_suspend and omap2430_runtime_suspend are never called in my setup, unless I revert this patch. I'm using musb with ethernet gadget; cable removal also doesn't suspend musb. It seems pm_runtime_put() that was removed was balancing out pm_runtime_get in musb_platform_init(), which is called at the beginning of musb_init_controller(). I think this pm_runtime_get() from musb_platform_init() should be moved to musb_init_controller() to stop confusing people and pm_runtime_put() needs to be brought back. I can do a patch if nobody objects. You mean have both get and put in musb_init_controller()? That's OK, it would be balanced then, but you would have to remove the pm_runtime_put() from omap2430_remove() that presumably was balancing the pm_runtime_get_sync() in omap2430_musb_init(). This was introduced in 7acc619[1], but it wasn't triggered in my setup until 18a2689[2] was merged to Linus' branch at point df0914[3]. IOW; when PM is working as it was supposed to. However, it seems most of the time this is used in a way that keeps the counter above 0, so nobody noticed. Also, it seems to depend on the configuration used in versions before 3.1, but not later (or in it). I found the problem by loading isp1704_charger before any usb gadgets: http://article.gmane.org/gmane.linux.kernel/1226122 isp1704_charger seems to be doing otg_io_read(), and it's perfectly normal that musb can be suspended at the time isp1704 decides to do this. I think proper fix is to wake up must on musb_ulpi_read() and musb_ulpi_write() instead. I guess so. If you send these patches I can give them a try. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: drop superfluous pm_runtime calls around musb_shutdown
On Thu, Jan 12, 2012 at 3:21 PM, Grazvydas Ignotas nota...@gmail.com wrote: Since commit 4f9edd2d7e8d usb: musb: Fix the crash issue during reboot musb_shutdown() does pm_runtime_get_sync/pm_runtime_put by itself, so this no longer needs to be done by the caller. Also, musb_exit_debugfs() doesn't access the device, so just drop those runtime_pm calls. I don't understand what is the relationship with 4f9edd2d7e8d and musb_shutdown(), it seems this patch is basically reverting 4f9edd2d7e8d. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: drop superfluous pm_runtime calls around musb_shutdown
On Thu, Jan 12, 2012 at 5:55 PM, Grazvydas Ignotas nota...@gmail.com wrote: On Thu, Jan 12, 2012 at 5:45 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Jan 12, 2012 at 3:21 PM, Grazvydas Ignotas nota...@gmail.com wrote: Since commit 4f9edd2d7e8d usb: musb: Fix the crash issue during reboot musb_shutdown() does pm_runtime_get_sync/pm_runtime_put by itself, so this no longer needs to be done by the caller. Also, musb_exit_debugfs() doesn't access the device, so just drop those runtime_pm calls. I don't understand what is the relationship with 4f9edd2d7e8d and musb_shutdown(), it seems this patch is basically reverting 4f9edd2d7e8d. No it's not, read my patch and it's description again. 4f9edd2d7e8d is patching musb_shutdown(), mine musb_remove(). Ohh, OK. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: It looks like snd-soc-rx51 only works as built-in, not as a module
On Sat, Dec 31, 2011 at 2:59 AM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Fri, Dec 30, 2011 at 10:00:55PM +0200, Felipe Contreras wrote: On Thu, Dec 29, 2011 at 11:54 PM, Mark Brown On Thu, Dec 29, 2011 at 11:51:11PM +0200, Felipe Contreras wrote: That should not be required, the modules should be loadable in any order. Yes, but udev loads snd_soc_tlv320aic3x, not snd-soc-rx51. That is compltelely orthogonal to what you were saying above about the ordering of module loading. Not really, because if both are compiled as modules, snd_soc_tlv320aic3x will always be loaded first (automatically by udev). Which is relevant because...? To repeat, the ordering of module loading is totally ortogonal as any ordering of modules is supported. You appear to be totally ignoring what I am writing here. One thing how things _should_ be, and another is how things actually _are_. Right now snd-soc-rx51 doesn't work if it's loaded before snd_soc_tlv320aic3x. Period. That means that *right now*, snd-soc-rx51 works fine with udev, because snd_soc_tlv320aic3x will be loaded at boot time. Now, if snd-soc-rx51 is converted and loaded automatically by udev as well, the issues might appear again. The reason the driver is not loaded automatically is that the OMAP machine drivers have not been converted to platform devices. Well, if that's the case then there's a real issue. There doesn't seem to be a MODULE_DEPENDS(), or anything like that. A MODULE_DEPENDS() would also be irrelevant here. Ok. snd-soc-rx51 seems to depend on snd_soc_tlv320aic3x through codec_dai_name, and codec_name, and so on. I don't know how this dependency is supposed to work out. All the drivers should load via the normal driver instantiation mechainsms and when they're all there they'll get matched together. Ok, I hope so. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: It looks like snd-soc-rx51 only works as built-in, not as a module
On Thu, Dec 29, 2011 at 11:54 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Dec 29, 2011 at 11:51:11PM +0200, Felipe Contreras wrote: On Thu, Dec 29, 2011 at 11:27 PM, Mark Brown On Thu, Dec 29, 2011 at 11:25:50PM +0200, Felipe Contreras wrote: On Thu, Dec 29, 2011 at 8:37 PM, Mark Brown On Sat, Dec 10, 2011 at 05:59:57PM +0200, Felipe Contreras wrote: It was not working for me, but it seems the problem was related to mdev/udev; snd_soc_tlv320aic3x has to be loaded before snd-soc-rx51. That should not be required, the modules should be loadable in any order. Are you sure? I recall having a similar discussion with Rusell King, and the conclusion is that certain modules are supposed to be loaded by udev at boot time, and it seems snd_soc_tlv320aic3x is one of them. I'm absolutely positive. ASoC supports loading the modules in any order. udev is supposed to load anything autoloadable at boot time, including I2C devices, but that's orthogonal to the order in which things actually get loaded - udev can randomly reorder things if it feels like it. Yes, but udev loads snd_soc_tlv320aic3x, not snd-soc-rx51. That is compltelely orthogonal to what you were saying above about the ordering of module loading. Not really, because if both are compiled as modules, snd_soc_tlv320aic3x will always be loaded first (automatically by udev). The reason the driver is not loaded automatically is that the OMAP machine drivers have not been converted to platform devices. Well, if that's the case then there's a real issue. There doesn't seem to be a MODULE_DEPENDS(), or anything like that. snd-soc-rx51 seems to depend on snd_soc_tlv320aic3x through codec_dai_name, and codec_name, and so on. I don't know how this dependency is supposed to work out. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: It looks like snd-soc-rx51 only works as built-in, not as a module
On Thu, Dec 29, 2011 at 8:37 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Sat, Dec 10, 2011 at 05:59:57PM +0200, Felipe Contreras wrote: It was not working for me, but it seems the problem was related to mdev/udev; snd_soc_tlv320aic3x has to be loaded before snd-soc-rx51. That should not be required, the modules should be loadable in any order. Are you sure? I recall having a similar discussion with Rusell King, and the conclusion is that certain modules are supposed to be loaded by udev at boot time, and it seems snd_soc_tlv320aic3x is one of them. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: It looks like snd-soc-rx51 only works as built-in, not as a module
On Thu, Dec 29, 2011 at 11:27 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Dec 29, 2011 at 11:25:50PM +0200, Felipe Contreras wrote: On Thu, Dec 29, 2011 at 8:37 PM, Mark Brown On Sat, Dec 10, 2011 at 05:59:57PM +0200, Felipe Contreras wrote: It was not working for me, but it seems the problem was related to mdev/udev; snd_soc_tlv320aic3x has to be loaded before snd-soc-rx51. That should not be required, the modules should be loadable in any order. Are you sure? I recall having a similar discussion with Rusell King, and the conclusion is that certain modules are supposed to be loaded by udev at boot time, and it seems snd_soc_tlv320aic3x is one of them. I'm absolutely positive. ASoC supports loading the modules in any order. udev is supposed to load anything autoloadable at boot time, including I2C devices, but that's orthogonal to the order in which things actually get loaded - udev can randomly reorder things if it feels like it. Yes, but udev loads snd_soc_tlv320aic3x, not snd-soc-rx51. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
On Sun, Dec 25, 2011 at 2:03 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Fri, Dec 23, 2011 at 11:04 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Which omap-iommu? The platform driver, or the device stuff? The device stuff is always built-in, but not the platform driver (drivers/iommu/omap-iommu.c), that can be a module. Both, I can't recall exactly when it changed (prior to being moved to drivers/iommu it could be built as a module), but now CONFIG_OMAP_IOMMU is boolean type. I see. Still, it looks like the proper way to use the API is to call _enable() on the platform driver. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
On Fri, Dec 23, 2011 at 6:30 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Mon, Dec 19, 2011 at 10:27 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations, reset and sysconfig handling. Removed clk handling during interrupt, given that in order to receive one, the device should be powered on in advance. Now doing pm_runtime_get/put on iommu_enable/disable so it doesn't rely on others to keep the clocks on. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-omap2/iommu2.c | 17 --- arch/arm/mach-omap2/omap-iommu.c | 1 - arch/arm/plat-omap/include/plat/iommu.h | 2 - arch/arm/plat-omap/include/plat/iommu2.h | 2 - drivers/iommu/omap-iommu.c | 44 - 5 files changed, 18 insertions(+), 48 deletions(-) Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections where the device should be active? omap_iommu_init is called on module init however omap_iommu_probe is called by driver instance (isp, iva), on probe pm_runtime_enable activates runtime for both isp and iva devices, one at a time. Yes, but omap_iommu_init() will *always* be called at boot time and will register the data for all the devices. There are 2 omap_iommu_init :/ Thought you were talking about the one in drivers/iommu/omap-iommu.c. If the 'iommu' module is never loaded, then the devices will remain active the whole time. oma-iommu is meant to be built-in as part of the kernel, there is no option for module anymore. Which omap-iommu? The platform driver, or the device stuff? The device stuff is always built-in, but not the platform driver (drivers/iommu/omap-iommu.c), that can be a module. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] arm: omap4: fix kconfig warning
On Thu, Dec 22, 2011 at 8:22 PM, Tony Lindgren t...@atomide.com wrote: * Felipe Contreras felipe.contre...@gmail.com [111219 16:11]: warning: (ARCH_OMAP4 ARCH_VEXPRESS_CA9X4) selects ARM_ERRATA_720789 which has unmet direct dependencies (CPU_V7 SMP) Thanks applying into fixes-non-critical-part3. Actually, please disregard this one, it should be handled by this: http://article.gmane.org/gmane.linux.ports.arm.kernel/141942 Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: fix pm_runtime mismatches
On Tue, Dec 20, 2011 at 1:19 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Dec 19, 2011 at 10:20:09PM +0200, Felipe Contreras wrote: On Mon, Dec 19, 2011 at 10:11 AM, Felipe Balbi ba...@ti.com wrote: On Fri, Dec 16, 2011 at 01:05:08AM +0200, Felipe Contreras wrote: Properly call pm_runtime_put() afer pm_runttime_get() on errors. Untested. sorry, but I'm not applying untested patches. I need someone to give a Tested-by. I'm pretty sure most people don't test the exit paths, but fine, don't apply them. Who needs power management anyway. if it wasn't tested you can't claim $SUBJECT makes PM magically work. It's clear from the code, no need to test to see that pm_runtime_get() is called but not pm_runtime_put(), thus the device remains active, thus PM would not work after that. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
On Fri, Dec 16, 2011 at 4:01 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote:
On Thu, Dec 15, 2011 at 6:39 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
Are you sure you are not missing something like:
.clk = cam_ick,
I believe in this case cam_ick is used as the main clock as it
supplies both functional and interface.
Are you sure?
+/* isp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
+ omap3xxx_l4_core__isp_mmu,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
+ .name = isp_mmu,
+ .class = omap3xxx_mmu_hwmod_class,
+ .mpu_irqs = omap3xxx_isp_mmu_irqs,
+ .main_clk = cam_ick,
It's not cam_fck?
AFAIK cam_fck doesn't exist in the code, and CAM_L3_ICK is used as
both ick/fck according to TRM.
Then maybe the code is wrong.
Look at the OMAP34xx documentation, it says that:
CAM_L3_ICLK - CAM_FCLK
CAM_L4_ICLK - CAM_ICLK
CAM_MCLK - CAM_MCLK
CSI2_96M_FCLK - CSI2_96M_FCLK
CAM_FCLK
Functional clock (L3 interconnect clock domain)
Functional clock domain.
CAM_ICLK
Interface clock (L4 interconnect clock domain)
Interface clock domain.
Maybe something like this:
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -2225,8 +2225,17 @@ static struct clk cam_mclk = {
.recalc = followparent_recalc,
};
+static struct clk cam_fck = {
+ .name = cam_fck,
+ .ops= clkops_omap2_dflt,
+ .parent = l3_ick,
+ .enable_reg = OMAP_CM_REGADDR(OMAP3430_CAM_MOD, CM_FCLKEN),
+ .enable_bit = OMAP3430_EN_CAM_SHIFT,
+ .clkdm_name = cam_clkdm,
+ .recalc = followparent_recalc,
+};
+
static struct clk cam_ick = {
- /* Handles both L3 and L4 clocks */
.name = cam_ick,
.ops= clkops_omap2_iclk_dflt,
.parent = l4_ick,
@@ -3364,6 +3373,7 @@ static struct omap_clk omap3xxx_clks[] = {
CLK(omapdss_dss, ick, dss_ick_3430es1,
CK_3430ES1),
CLK(omapdss_dss, ick, dss_ick_3430es2,
CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
CLK(NULL, cam_mclk, cam_mclk, CK_34XX | CK_36XX),
+ CLK(NULL, cam_fck, cam_fck, CK_34XX | CK_36XX),
CLK(NULL, cam_ick, cam_ick, CK_34XX | CK_36XX),
CLK(NULL, csi2_96m_fck, csi2_96m_fck, CK_34XX | CK_36XX),
CLK(NULL, usbhost_120m_fck, usbhost_120m_fck,
CK_3430ES2PLUS | CK_AM35XX | CK_36XX),
Also, I find this chunk peculiar in drivers/media/video/omap3isp/isp.c:
static const char *isp_clocks[] = {
cam_ick,
cam_mclk,
dpll4_m5_ck,
csi2_96m_fck,
l3_ick,
};
Looks like the driver is manually calling clk_get() and clk_put() for
the i3_ick, where I guess the clock framework is supposed to do
that... It's almost as if somebody forgot a dependency somewhere.
+ .dev_attr = isp_mmu_dev_attr,
+ .slaves = omap3xxx_isp_mmu_slaves,
+ .slaves_cnt = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
+ .flags = HWMOD_NO_IDLEST,
+};
Most of the stuff I see the hwmods is .main_lock = foo_fck, slave
.clk = foo_ick. Maybe that explains the irq issues you get.
I see irq issues with iva hwmod because tidspbridge doesn't use iommu
API yet, so if you enable both the mmu hwmod and tidspbridge own mmu
implementation there will be some conflicts.
I didn't see isp issues though, but I didn't went more than
booting/enabling with isp mmu.
This is what you said:
Removed clk handling during interrupt, given that in order to receive one,
the device should be powered on in advance.
I'm not sure how this clock stuff works, but I'm guessing the device
is supposed to go to sleep at some points in time, and with your patch
OMAP3/4: iommu: adapt to runtime pm it won't, as long as the module
is loaded, unless I'm missing something.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
On Fri, Dec 16, 2011 at 5:18 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Thu, Dec 15, 2011 at 6:53 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations, reset and sysconfig handling. Removed clk handling during interrupt, given that in order to receive one, the device should be powered on in advance. Now doing pm_runtime_get/put on iommu_enable/disable so it doesn't rely on others to keep the clocks on. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-omap2/iommu2.c | 17 --- arch/arm/mach-omap2/omap-iommu.c | 1 - arch/arm/plat-omap/include/plat/iommu.h | 2 - arch/arm/plat-omap/include/plat/iommu2.h | 2 - drivers/iommu/omap-iommu.c | 44 - 5 files changed, 18 insertions(+), 48 deletions(-) Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections where the device should be active? omap_iommu_init is called on module init however omap_iommu_probe is called by driver instance (isp, iva), on probe pm_runtime_enable activates runtime for both isp and iva devices, one at a time. Yes, but omap_iommu_init() will *always* be called at boot time and will register the data for all the devices. If the 'iommu' module is never loaded, then the devices will remain active the whole time. Maybe Paul or somebody with better knowledge of the pm runtime framework can clarify this. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ZOOM2 doesn't boot on upstream kernel
On Fri, Dec 16, 2011 at 10:53 PM, Tony Lindgren t...@atomide.com wrote: You should use ttyO for the omap ports, but zoom debug board has external 8250 ftdi ports, so those show up as ttyS and not ttyO. Plus that was changed on 2.6.36 (or after?) and he is using 2.6.35. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: musb: fix pm_runtime mismatch
From: Felipe Contreras felipe.contre...@gmail.com In musb_init_controller() there's a pm_runtime_put(), but there's no pm_runtime_get(), which creates a mismatch that causes the driver to sleep when it shouldn't. This was introduced in 7acc619[1], but it wasn't triggered in my setup until 18a2689[2] was merged to Linus' branch at point df0914[3]. IOW; when PM is working as it was supposed to. However, it seems most of the time this is used in a way that keeps the counter above 0, so nobody noticed. Also, it seems to depend on the configuration used in versions before 3.1, but not later (or in it). I found the problem by loading isp1704_charger before any usb gadgets: http://article.gmane.org/gmane.linux.kernel/1226122 All versions after 2.6.39 are affected. [1] usb: musb: Idle path retention and offmode support for OMAP3 [2] OMAP2+: musb: hwmod adaptation for musb registration [3] Merge branch 'omap-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6 Cc: sta...@vger.kernel.org Cc: Hema HK hem...@ti.com Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/usb/musb/musb_core.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index b63ab15..920f04e 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -2012,8 +2012,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) if (status 0) goto fail3; - pm_runtime_put(musb-controller); - status = musb_init_debugfs(musb); if (status 0) goto fail4; -- 1.7.8 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] musb: omap2430: avoid pm_runtime_disable()
From: Felipe Contreras felipe.contre...@gmail.com These are handled by drivers core, and in a way that doesn't wake up the devices. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/usb/musb/omap2430.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index 521d806..66c1b8d 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -328,7 +328,6 @@ static int omap2430_musb_init(struct musb *musb) return 0; err1: - pm_runtime_disable(dev); return status; } @@ -472,7 +471,6 @@ static int __exit omap2430_remove(struct platform_device *pdev) platform_device_del(glue-musb); platform_device_put(glue-musb); pm_runtime_put(pdev-dev); - pm_runtime_disable(pdev-dev); kfree(glue); return 0; -- 1.7.8 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: fix pm_runtime mismatches
On Mon, Dec 19, 2011 at 10:11 AM, Felipe Balbi ba...@ti.com wrote: On Fri, Dec 16, 2011 at 01:05:08AM +0200, Felipe Contreras wrote: Properly call pm_runtime_put() afer pm_runttime_get() on errors. Untested. sorry, but I'm not applying untested patches. I need someone to give a Tested-by. I'm pretty sure most people don't test the exit paths, but fine, don't apply them. Who needs power management anyway. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] usb: musb: fix pm_runtime mismatches
From: Felipe Contreras felipe.contre...@gmail.com Properly call pm_runtime_put() afer pm_runttime_get() on errors. Comments from Alan Stern. Untested. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/usb/musb/musb_gadget.c |1 + drivers/usb/musb/omap2430.c|1 + 2 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index 922148f..4d6a37a 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1960,6 +1960,7 @@ static int musb_gadget_start(struct usb_gadget *g, err2: if (!is_otg_enabled(musb)) musb_stop(musb); + pm_runtime_put_sync(musb-controller); err0: return retval; } diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index ba85f27..1a5d45e 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -389,6 +389,7 @@ static int omap2430_musb_exit(struct musb *musb) omap2430_low_level_exit(musb); otg_put_transceiver(musb-xceiv); + pm_runtime_put_sync(musb-controller); return 0; } -- 1.7.8 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] usb: musb: remove a bit of indentation
From: Felipe Contreras felipe.contre...@gmail.com
And use dev instead of musb-controller.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/omap2430.c | 27 +--
1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 6f574ec..521d806 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -344,20 +344,19 @@ static void omap2430_musb_enable(struct musb *musb)
case USB_EVENT_ID:
otg_init(musb-xceiv);
- if (data-interface_type == MUSB_INTERFACE_UTMI) {
- devctl = musb_readb(musb-mregs, MUSB_DEVCTL);
- /* start the session */
- devctl |= MUSB_DEVCTL_SESSION;
- musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
- while (musb_readb(musb-mregs, MUSB_DEVCTL)
- MUSB_DEVCTL_BDEVICE) {
- cpu_relax();
-
- if (time_after(jiffies, timeout)) {
- dev_err(musb-controller,
- configured as A device timeout);
- break;
- }
+ if (data-interface_type != MUSB_INTERFACE_UTMI)
+ break;
+ devctl = musb_readb(musb-mregs, MUSB_DEVCTL);
+ /* start the session */
+ devctl |= MUSB_DEVCTL_SESSION;
+ musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
+ while (musb_readb(musb-mregs, MUSB_DEVCTL)
+ MUSB_DEVCTL_BDEVICE) {
+ cpu_relax();
+
+ if (time_after(jiffies, timeout)) {
+ dev_err(dev, configured as A device timeout);
+ break;
}
}
break;
--
1.7.8
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] usb: musb: trivial cleanup
From: Felipe Contreras felipe.contre...@gmail.com
enabled driver || !enabled can be simplified to !enabled || driver.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/omap2430.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 1a5d45e..6f574ec 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -236,13 +236,7 @@ static int musb_otg_notifications(struct notifier_block
*nb,
case USB_EVENT_ID:
dev_dbg(musb-controller, ID GND\n);
- if (is_otg_enabled(musb)) {
- if (musb-gadget_driver) {
- pm_runtime_get_sync(musb-controller);
- otg_init(musb-xceiv);
- omap2430_musb_set_vbus(musb, 1);
- }
- } else {
+ if (!is_otg_enabled(musb) || musb-gadget_driver) {
pm_runtime_get_sync(musb-controller);
otg_init(musb-xceiv);
omap2430_musb_set_vbus(musb, 1);
--
1.7.8
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] musb fixes and cleanups
Hi, Here's a few cleanups and possible fixes. Nothing major. In v2 I added comments from Alan Stern, and another possible fix by removing some pm_runtime_disable() calls. Felipe Contreras (4): usb: musb: fix pm_runtime mismatches usb: musb: trivial cleanup usb: musb: remove a bit of indentation musb: omap2430: avoid pm_runtime_disable() drivers/usb/musb/musb_gadget.c |1 + drivers/usb/musb/omap2430.c| 38 +++--- 2 files changed, 16 insertions(+), 23 deletions(-) -- 1.7.8 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: fix pm_runtime mismatches
On Fri, Dec 16, 2011 at 5:30 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 16 Dec 2011, Felipe Contreras wrote: Should these calls be pm_runtime_put_sync() instead of pm_runtime_put()? I don't see why... The thing failed, it's not going to be used any more so better let PM deactivate the device. It's not a big deal. The difference is that pm_runtime_put_sync() avoids an extra context switch when suspending the device. A minor optimization, that's all. Ok, I used what most of the code was using, but I've trusted you and updated the patches. It would probably make sense to do the same in other places. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/10] arm: omap4: fix kconfig warning
warning: (ARCH_OMAP4 ARCH_VEXPRESS_CA9X4) selects ARM_ERRATA_720789 which has unmet direct dependencies (CPU_V7 SMP) Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- arch/arm/mach-omap2/Kconfig |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index e1293aa..eb7c3ac 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -46,7 +46,7 @@ config ARCH_OMAP4 select LOCAL_TIMERS if SMP select PL310_ERRATA_588369 select PL310_ERRATA_727915 - select ARM_ERRATA_720789 + select ARM_ERRATA_720789 if SMP select ARCH_HAS_OPP select PM_OPP if PM select USB_ARCH_HAS_EHCI -- 1.7.8.rc1.14.g248db -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/10] arm: omap2: fix regulator warnings
warning: (MACH_OMAP_ZOOM2 MACH_OMAP_ZOOM3 MACH_OMAP_4430SDP MACH_OMAP4_PANDA TPS6105X) selects REGULATOR_FIXED_VOLTAGE which has unmet direct dependencies (REGULATOR) Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- arch/arm/mach-omap2/Kconfig | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index dbecb9a..04734b5 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -205,7 +205,7 @@ config MACH_OMAP3_PANDORA depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB - select REGULATOR_FIXED_VOLTAGE + select REGULATOR_FIXED_VOLTAGE if REGULATOR config MACH_OMAP3_TOUCHBOOK bool OMAP3 Touch Book @@ -256,7 +256,7 @@ config MACH_OMAP_ZOOM2 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE - select REGULATOR_FIXED_VOLTAGE + select REGULATOR_FIXED_VOLTAGE if REGULATOR config MACH_OMAP_ZOOM3 bool OMAP3630 Zoom3 board @@ -266,7 +266,7 @@ config MACH_OMAP_ZOOM3 select SERIAL_8250 select SERIAL_CORE_CONSOLE select SERIAL_8250_CONSOLE - select REGULATOR_FIXED_VOLTAGE + select REGULATOR_FIXED_VOLTAGE if REGULATOR config MACH_CM_T35 bool CompuLab CM-T35/CM-T3730 modules @@ -320,7 +320,7 @@ config MACH_OMAP_4430SDP depends on ARCH_OMAP4 select OMAP_PACKAGE_CBL select OMAP_PACKAGE_CBS - select REGULATOR_FIXED_VOLTAGE + select REGULATOR_FIXED_VOLTAGE if REGULATOR config MACH_OMAP4_PANDA bool OMAP4 Panda Board @@ -328,7 +328,7 @@ config MACH_OMAP4_PANDA depends on ARCH_OMAP4 select OMAP_PACKAGE_CBL select OMAP_PACKAGE_CBS - select REGULATOR_FIXED_VOLTAGE + select REGULATOR_FIXED_VOLTAGE if REGULATOR config OMAP3_EMU bool OMAP3 debugging peripherals -- 1.7.8.rc1.14.g248db -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/10] arm: omap2: fix omap3 touchbook kconfig warning
warning: (MACH_OMAP3_TOUCHBOOK DRM_RADEON_KMS DRM_I915 STUB_POULSBO FB_BACKLIGHT USB_APPLEDISPLAY FB_OLPC_DCON ASUS_LAPTOP SONY_LAPTOP THINKPAD_ACPI EEEPC_LAPTOP ACPI_ASUS ACPI_CMPC SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM BACKLIGHT_LCD_SUPPORT) A lot of boards need BACKLIGHT_CLASS_DEVICE for the framebuffers to work, but it's not *needed* for the device itself. It might be nice to enable it by default somewhoe if graphics stuff is enabled, but that's another story. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- arch/arm/mach-omap2/Kconfig |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index eb7c3ac..dbecb9a 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -211,7 +211,6 @@ config MACH_OMAP3_TOUCHBOOK bool OMAP3 Touch Book depends on ARCH_OMAP3 default y - select BACKLIGHT_CLASS_DEVICE config MACH_OMAP_3430SDP bool OMAP 3430 SDP board -- 1.7.8.rc1.14.g248db -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue with isp1704 loaded too soon
On Thu, Dec 15, 2011 at 1:15 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Dec 13, 2011 at 11:19 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Dec 12, 2011 at 9:09 AM, Jarkko Nikula jarkko.nik...@bitmer.com wrote: Indeed yes. I checked that in 3.0 it still works but not in 3.1 so some non isp1704_charger change has broke it as there hasn't been changes on it. Actually it's broken in 3.0 as well, try this configuration: # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_PERIPHERAL is not set CONFIG_USB_MUSB_OTG=y CONFIG_USB_GADGET_MUSB_HDRC=y CONFIG_USB_MUSB_HDRC_HCD=y CONFIG_USB_GADGET_SELECTED=y # CONFIG_USB_GADGET_FUSB300 is not set # CONFIG_USB_GADGET_OMAP is not set # CONFIG_USB_GADGET_R8A66597 is not set # CONFIG_USB_GADGET_PXA_U2O is not set # CONFIG_USB_GADGET_M66592 is not set # CONFIG_USB_GADGET_DUMMY_HCD is not set I will try to find where the issues started to happen, but it's a bit difficult because all this USB Kconfig stuff is completely messed up. *Sigh* Ok, I made some progress. The issue appeared on 2.6.39, specifically, on commit 0df0914, which is a merge from Linus. The issue doesn't appear in any of the two parents of that merge. I manually merged various commits, and the first one that triggered the problem is 18a2689, but the commit itself is not the problem but something that was already merged to Linus' tree, specifically at point 6899608. IOW, it's a mixture of two changes that appear together at point, 0df0914, one of them is 18a2689, the other one I still don't know. Unless you can find what's wrong with this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blobdiff;f=arch/arm/mach-omap2/usb-musb.c;h=a9d4d143086d73463b97c093cc9f022de8e1d679;hp=9788b4941857f40d50d48fa6470ee14e179bc373;hb=18a2689;hpb=273ff8c3bc04cf358c131f49ee7a11efa7ec73d7 I guess the safest route is to continue with the manual merges to find the second relevant commit =/ Found it. So, the first commit is this one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=18a2689 Which needs this one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=870ea2b But it's only triggered with this other one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=7acc619 Seems to be some power management stuff, and it's only triggered with certain configurations. And this concludes the most tedious git bisect in history =/ Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue with isp1704 loaded too soon
On Thu, Dec 15, 2011 at 7:51 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Dec 15, 2011 at 1:15 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Tue, Dec 13, 2011 at 11:19 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Dec 12, 2011 at 9:09 AM, Jarkko Nikula jarkko.nik...@bitmer.com wrote: Indeed yes. I checked that in 3.0 it still works but not in 3.1 so some non isp1704_charger change has broke it as there hasn't been changes on it. Actually it's broken in 3.0 as well, try this configuration: # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_PERIPHERAL is not set CONFIG_USB_MUSB_OTG=y CONFIG_USB_GADGET_MUSB_HDRC=y CONFIG_USB_MUSB_HDRC_HCD=y CONFIG_USB_GADGET_SELECTED=y # CONFIG_USB_GADGET_FUSB300 is not set # CONFIG_USB_GADGET_OMAP is not set # CONFIG_USB_GADGET_R8A66597 is not set # CONFIG_USB_GADGET_PXA_U2O is not set # CONFIG_USB_GADGET_M66592 is not set # CONFIG_USB_GADGET_DUMMY_HCD is not set I will try to find where the issues started to happen, but it's a bit difficult because all this USB Kconfig stuff is completely messed up. *Sigh* Ok, I made some progress. The issue appeared on 2.6.39, specifically, on commit 0df0914, which is a merge from Linus. The issue doesn't appear in any of the two parents of that merge. I manually merged various commits, and the first one that triggered the problem is 18a2689, but the commit itself is not the problem but something that was already merged to Linus' tree, specifically at point 6899608. IOW, it's a mixture of two changes that appear together at point, 0df0914, one of them is 18a2689, the other one I still don't know. Unless you can find what's wrong with this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blobdiff;f=arch/arm/mach-omap2/usb-musb.c;h=a9d4d143086d73463b97c093cc9f022de8e1d679;hp=9788b4941857f40d50d48fa6470ee14e179bc373;hb=18a2689;hpb=273ff8c3bc04cf358c131f49ee7a11efa7ec73d7 I guess the safest route is to continue with the manual merges to find the second relevant commit =/ Found it. So, the first commit is this one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=18a2689 Which needs this one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=870ea2b But it's only triggered with this other one: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=7acc619 Seems to be some power management stuff, and it's only triggered with certain configurations. And this concludes the most tedious git bisect in history =/ It seems I found the fix: http://mid.gmane.org/1323988934-11350-1-git-send-email-felipe.contre...@gmail.com -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] musb fixes and cleanups
Hi, Here's a few cleanups and a possible fix. Nothing major. Felipe Contreras (3): usb: musb: fix pm_runtime mismatches usb: musb: trivial cleanup usb: musb: remove a bit of indentation drivers/usb/musb/musb_gadget.c |1 + drivers/usb/musb/omap2430.c| 36 +++- 2 files changed, 16 insertions(+), 21 deletions(-) -- 1.7.8.rc1.14.g248db -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usb: musb: fix pm_runtime mismatches
Properly call pm_runtime_put() afer pm_runttime_get() on errors.
Untested.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/musb_gadget.c |1 +
drivers/usb/musb/omap2430.c|1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 922148f..95bfd2d 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1960,6 +1960,7 @@ static int musb_gadget_start(struct usb_gadget *g,
err2:
if (!is_otg_enabled(musb))
musb_stop(musb);
+ pm_runtime_put(musb-controller);
err0:
return retval;
}
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index ba85f27..4edfb91 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -385,6 +385,7 @@ static void omap2430_musb_disable(struct musb *musb)
static int omap2430_musb_exit(struct musb *musb)
{
+ pm_runtime_put(musb-controller);
del_timer_sync(musb_idle_timer);
omap2430_low_level_exit(musb);
--
1.7.8.rc1.14.g248db
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] usb: musb: trivial cleanup
enabled driver || !enabled can be simplified to !enabled || driver.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/omap2430.c |8 +---
1 files changed, 1 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 4edfb91..a615538 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -236,13 +236,7 @@ static int musb_otg_notifications(struct notifier_block
*nb,
case USB_EVENT_ID:
dev_dbg(musb-controller, ID GND\n);
- if (is_otg_enabled(musb)) {
- if (musb-gadget_driver) {
- pm_runtime_get_sync(musb-controller);
- otg_init(musb-xceiv);
- omap2430_musb_set_vbus(musb, 1);
- }
- } else {
+ if (!is_otg_enabled(musb) || musb-gadget_driver) {
pm_runtime_get_sync(musb-controller);
otg_init(musb-xceiv);
omap2430_musb_set_vbus(musb, 1);
--
1.7.8.rc1.14.g248db
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] usb: musb: remove a bit of indentation
And use dev instead of musb-controller.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/omap2430.c | 27 +--
1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index a615538..31fc8a2 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -344,20 +344,19 @@ static void omap2430_musb_enable(struct musb *musb)
case USB_EVENT_ID:
otg_init(musb-xceiv);
- if (data-interface_type == MUSB_INTERFACE_UTMI) {
- devctl = musb_readb(musb-mregs, MUSB_DEVCTL);
- /* start the session */
- devctl |= MUSB_DEVCTL_SESSION;
- musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
- while (musb_readb(musb-mregs, MUSB_DEVCTL)
- MUSB_DEVCTL_BDEVICE) {
- cpu_relax();
-
- if (time_after(jiffies, timeout)) {
- dev_err(musb-controller,
- configured as A device timeout);
- break;
- }
+ if (data-interface_type != MUSB_INTERFACE_UTMI)
+ break;
+ devctl = musb_readb(musb-mregs, MUSB_DEVCTL);
+ /* start the session */
+ devctl |= MUSB_DEVCTL_SESSION;
+ musb_writeb(musb-mregs, MUSB_DEVCTL, devctl);
+ while (musb_readb(musb-mregs, MUSB_DEVCTL)
+ MUSB_DEVCTL_BDEVICE) {
+ cpu_relax();
+
+ if (time_after(jiffies, timeout)) {
+ dev_err(dev, configured as A device timeout);
+ break;
}
}
break;
--
1.7.8.rc1.14.g248db
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
: release_mem_region(res-start, resource_size(res)); err_mem: - clk_put(obj-clk); -err_clk: kfree(obj); return err; } @@ -996,7 +987,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev) release_mem_region(res-start, resource_size(res)); iounmap(obj-regbase); - clk_put(obj-clk); + pm_runtime_disable(obj-dev); I'm not sure if this is needed; you want to resume the driver? AFAICS kfree will take care of that _without_ resuming. dev_info(pdev-dev, %s removed\n, obj-name); kfree(obj); return 0; Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] OMAP3: hwmod data: add mmu data for iva and isp
On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote:
+/* l4_core - isp mmu */
+static struct omap_hwmod_ocp_if omap3xxx_l4_core__isp_mmu = {
+ .master = omap3xxx_l4_core_hwmod,
+ .slave = omap3xxx_isp_mmu_hwmod,
+ .addr = omap3xxx_isp_mmu_addrs,
+ .user = OCP_USER_MPU | OCP_USER_SDMA,
+};
Are you sure you are not missing something like:
.clk = cam_ick,
+/* isp mmu slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_isp_mmu_slaves[] = {
+ omap3xxx_l4_core__isp_mmu,
+};
+
+static struct omap_hwmod omap3xxx_isp_mmu_hwmod = {
+ .name = isp_mmu,
+ .class = omap3xxx_mmu_hwmod_class,
+ .mpu_irqs = omap3xxx_isp_mmu_irqs,
+ .main_clk = cam_ick,
It's not cam_fck?
+ .dev_attr = isp_mmu_dev_attr,
+ .slaves = omap3xxx_isp_mmu_slaves,
+ .slaves_cnt = ARRAY_SIZE(omap3xxx_isp_mmu_slaves),
+ .flags = HWMOD_NO_IDLEST,
+};
Most of the stuff I see the hwmods is .main_lock = foo_fck, slave
.clk = foo_ick. Maybe that explains the irq issues you get.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] OMAP3/4: iommu: migrate to hwmod framework
On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Use hwmod data and device attributes to build and register an omap device for iommu driver. - Update the naming convention in isp module. - Remove unneeded check for number of resources, as this is now handled by omap_device and prevents driver from loading. - Now unused, remove platform device and resource data, handling of sysconfig register for softreset purposes, use default latency structure. Looks good to me. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] OMAP3/4: iommu: adapt to runtime pm
On Thu, Dec 15, 2011 at 6:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations, reset and sysconfig handling. Removed clk handling during interrupt, given that in order to receive one, the device should be powered on in advance. Now doing pm_runtime_get/put on iommu_enable/disable so it doesn't rely on others to keep the clocks on. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-omap2/iommu2.c | 17 --- arch/arm/mach-omap2/omap-iommu.c | 1 - arch/arm/plat-omap/include/plat/iommu.h | 2 - arch/arm/plat-omap/include/plat/iommu2.h | 2 - drivers/iommu/omap-iommu.c | 44 - 5 files changed, 18 insertions(+), 48 deletions(-) Shouldn't pm_runtime_enable() be called in omap_iommu_init(), and omap_iommu_probe() call pm_runtime_get_sync()/put() on the sections where the device should be active? Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: problem with twl4030 GPIO irqs
On Fri, Dec 16, 2011 at 2:03 AM, Grazvydas Ignotas nota...@gmail.com wrote: I'm running 3.2-rc5 on pandora with Neil's twl patches on top, and omap_hsmmc driver fails to probe: [ 3.491394] omap_hsmmc omap_hsmmc.0: context was not lost [ 3.497161] omap_hsmmc omap_hsmmc.0: enabled [ 3.715270] omap_hsmmc omap_hsmmc.0: Unable to grab MMC CD IRQ Pandora is using twl4030 GPIOs for card detect. From what I tried to debug, omap_hsmmc is failing after call to request_irq() on card detect irq with -EINVAL, irq number is 384 which is in range of other twl4030 irqs. It looks like request_irq() is calling request_threaded_irq() on ARM with NULL thread_fn, which in turn calls __setup_irq() that calls irq_settings_is_nested_thread() that results with true. As this nested flag is set it checks thread_fn, but that is NULL causing -EINVAL final result. Could someone who knows irq code better look at this? I think this should reproduce on beagle too. Basically all the modules of twl4030 should be converted to use request_threaded_irq(), if they are not, they will fail, and that's good, as trying to workaround the issue would just cause more trouble. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: musb: fix pm_runtime mismatches
On Fri, Dec 16, 2011 at 1:22 AM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 16 Dec 2011, Felipe Contreras wrote:
Properly call pm_runtime_put() afer pm_runttime_get() on errors.
Untested.
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
drivers/usb/musb/musb_gadget.c | 1 +
drivers/usb/musb/omap2430.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 922148f..95bfd2d 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1960,6 +1960,7 @@ static int musb_gadget_start(struct usb_gadget *g,
err2:
if (!is_otg_enabled(musb))
musb_stop(musb);
+ pm_runtime_put(musb-controller);
err0:
return retval;
}
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index ba85f27..4edfb91 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -385,6 +385,7 @@ static void omap2430_musb_disable(struct musb *musb)
static int omap2430_musb_exit(struct musb *musb)
{
+ pm_runtime_put(musb-controller);
del_timer_sync(musb_idle_timer);
omap2430_low_level_exit(musb);
Should these calls be pm_runtime_put_sync() instead of
pm_runtime_put()?
I don't see why... The thing failed, it's not going to be used any
more so better let PM deactivate the device. But now that you point
this out, pm_runtime_put() on omap2430_musb_exit() should probably go
at the end of the function, after all the actions have been done.
I'll send an updated patch tomorrow, if there are no further comments.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Interrupt issue in twl4030_keypad
On Mon, Dec 12, 2011 at 11:34 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Dec 12, 2011 at 10:55:11PM +0200, Felipe Contreras wrote: Samuel assumed it was ok and, like I said above, it worked for my simple GPIO usecase with beagleboard. Well, for 3.2 I think the situation is fine, but that's not what I'm talking about. About your GPIO tests, are you sure you got the interrupts through twl4030 irq handling? Card detect GPIO on beagleboard is a TWL GPIO. Checking that removing/adding the card actually triggers an IRQ, should be enough. No? Perhaps the reason that worked is because drivers/mmc/host/omap_hsmmc.c is not requesting a threaded IRQ, so the action handler was able to run while in handle_twl4030_pih() was still running (handle_nested_irq), but everything else that was using request_threaded_irq() didn't have an action handler, but a thread_fn(), so they were scheduled to run after handle_twl4030_pih(). That's also why you didn't notice the endless loop while pressing a key. The lesson to learn from this is; don't mix threaded and non-threaded irq handling. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Revert ARM: RX-51: Enable isp1704 power on/off
On Wed, Dec 14, 2011 at 9:30 AM, Felipe Balbi ba...@ti.com wrote: On Wed, Dec 14, 2011 at 01:02:09AM +0200, Felipe Contreras wrote: On Wed, Dec 14, 2011 at 12:53 AM, Felipe Balbi ba...@ti.com wrote: patches are welcome The were not last times: http://mid.gmane.org/1288656853-4625-1-git-send-email-felipe.contre...@gmail.com At least [1] will not apply anymore. Most of that stuff has been cleaned up after I introduced the UDC class/core driver. There are still lots of things to fix up, but it won't happen overnight. Specially if people are more willing to complain than to patch. Yes, the situation is much better, but these patches still apply on usb core, or at least the same concept. Anyway, I have the new trivial patches, I'll send them soon. [2] Makes no sense whatsoever. I have been fighting a lot against these ARCH dependencies. It's generally used to hide some moronic constructs relying on plat/* or mach/* headers. Most of the time, it's just to be able to access e.g. machine_is_*, cpu_is_* or omap_ctrl_read* and the like. Also, the first step for having a single ARM zImage is to get rid of those ARCH dependencies; we need to be able to compile modules on all ARCHes (even x86 for that matter) because: Read the patch carefully, it doesn't add any new dependencies, it just shuffles them around, I already explained that. And the rest of the stuff adds *defaults*, not dependencies, so you can still build in x86. So, if those are the only patches you can provide, please refrain from doing so as it will only take time reviewing. Instead, help really cleaning up the problems, make sure drivers compile on other ARCHes, make sure you don't include plat/* or mach/* on drivers, make sure you help reviewing patches which are coming in. Some of these changes have already been ack'ed, I just need to split them. The rest, I guess I have to push it upstairs to Linus so he can say if it does make sense to make builds more difficult for the users, and keep the size of the current defconfigs from decreasing. http://mid.gmane.org/1287482608-11320-1-git-send-email-felipe.contre...@gmail.com No such article It's there. Here's the direct link: http://article.gmane.org/gmane.linux.kernel/1050608 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Issue with isp1704 loaded too soon
On Tue, Dec 13, 2011 at 11:19 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Dec 12, 2011 at 9:09 AM, Jarkko Nikula jarkko.nik...@bitmer.com wrote: Indeed yes. I checked that in 3.0 it still works but not in 3.1 so some non isp1704_charger change has broke it as there hasn't been changes on it. Actually it's broken in 3.0 as well, try this configuration: # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_PERIPHERAL is not set CONFIG_USB_MUSB_OTG=y CONFIG_USB_GADGET_MUSB_HDRC=y CONFIG_USB_MUSB_HDRC_HCD=y CONFIG_USB_GADGET_SELECTED=y # CONFIG_USB_GADGET_FUSB300 is not set # CONFIG_USB_GADGET_OMAP is not set # CONFIG_USB_GADGET_R8A66597 is not set # CONFIG_USB_GADGET_PXA_U2O is not set # CONFIG_USB_GADGET_M66592 is not set # CONFIG_USB_GADGET_DUMMY_HCD is not set I will try to find where the issues started to happen, but it's a bit difficult because all this USB Kconfig stuff is completely messed up. *Sigh* Ok, I made some progress. The issue appeared on 2.6.39, specifically, on commit 0df0914, which is a merge from Linus. The issue doesn't appear in any of the two parents of that merge. I manually merged various commits, and the first one that triggered the problem is 18a2689, but the commit itself is not the problem but something that was already merged to Linus' tree, specifically at point 6899608. IOW, it's a mixture of two changes that appear together at point, 0df0914, one of them is 18a2689, the other one I still don't know. Unless you can find what's wrong with this commit: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blobdiff;f=arch/arm/mach-omap2/usb-musb.c;h=a9d4d143086d73463b97c093cc9f022de8e1d679;hp=9788b4941857f40d50d48fa6470ee14e179bc373;hb=18a2689;hpb=273ff8c3bc04cf358c131f49ee7a11efa7ec73d7 I guess the safest route is to continue with the manual merges to find the second relevant commit =/ -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] fixes for twl4030-irq in mainline
Hi, On Mon, Dec 12, 2011 at 7:38 PM, Samuel Ortiz sa...@linux.intel.com wrote: On Sun, Nov 27, 2011 at 07:17:41AM +1100, NeilBrown wrote: The recent tidying up of twl4030-irq seems to have left it broken. At least it doesn't work for me on my gta04 (www.gta04.org). The first interrupt from the device freezes the whole system (by being constantly delivered) The following 4 patches make it work for me and addresses some other less critical issues like a typo in a comment :-) Thanks, I applied all 4 of them. Did you apply them for 3.2 or 3.3? Without the first patch any system that has a twl4030 chip will immediately hang on the first interrupt, and many functions of twl4030 will just not work without the second one. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: rx51: initialize isp1704 properly
On Mon, Dec 12, 2011 at 9:44 PM, Tony Lindgren t...@atomide.com wrote: * Felipe Contreras felipe.contre...@gmail.com [111208 11:52]: On Thu, Dec 8, 2011 at 4:40 PM, Heikki Krogerus kro...@gmail.com wrote: On Wed, Dec 07, 2011 at 09:56:56PM +0200, Felipe Contreras wrote: From: Heikki Krogerus kro...@gmail.com I would prefer that you change this so this is from you, like it actually is. You can mention me in the commit message if you like. Well, you came with the fix, I just typed it down, but as you wish :) Without this USB networking doesn't work as the link is never detected. What this changes is, it leaves the transceiver powered by default instead of setting it into power off mode. So not only USB networking is affected by this. If isp1704_charger is not enabled with RX51, USB does not work at all. Please change the comment. I've updated the patch. Sorry what's the deal with this one? Is this the one to use or is there some other patch? Yeap, there's v2 of this one: 1323375780-13190-1-git-send-email-felipe.contre...@nokia.com Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Revert ARM: RX-51: Enable isp1704 power on/off
On Mon, Dec 12, 2011 at 9:09 AM, Jarkko Nikula jarkko.nik...@bitmer.com wrote: Indeed yes. I checked that in 3.0 it still works but not in 3.1 so some non isp1704_charger change has broke it as there hasn't been changes on it. Actually it's broken in 3.0 as well, try this configuration: # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_PERIPHERAL is not set CONFIG_USB_MUSB_OTG=y CONFIG_USB_GADGET_MUSB_HDRC=y CONFIG_USB_MUSB_HDRC_HCD=y CONFIG_USB_GADGET_SELECTED=y # CONFIG_USB_GADGET_FUSB300 is not set # CONFIG_USB_GADGET_OMAP is not set # CONFIG_USB_GADGET_R8A66597 is not set # CONFIG_USB_GADGET_PXA_U2O is not set # CONFIG_USB_GADGET_M66592 is not set # CONFIG_USB_GADGET_DUMMY_HCD is not set I will try to find where the issues started to happen, but it's a bit difficult because all this USB Kconfig stuff is completely messed up. *Sigh* -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Revert ARM: RX-51: Enable isp1704 power on/off
On Wed, Dec 14, 2011 at 12:53 AM, Felipe Balbi ba...@ti.com wrote: On Tue, Dec 13, 2011 at 11:19:52PM +0200, Felipe Contreras wrote: On Mon, Dec 12, 2011 at 9:09 AM, Jarkko Nikula jarkko.nik...@bitmer.com wrote: Indeed yes. I checked that in 3.0 it still works but not in 3.1 so some non isp1704_charger change has broke it as there hasn't been changes on it. Actually it's broken in 3.0 as well, try this configuration: # CONFIG_USB_MUSB_HOST is not set # CONFIG_USB_MUSB_PERIPHERAL is not set CONFIG_USB_MUSB_OTG=y CONFIG_USB_GADGET_MUSB_HDRC=y CONFIG_USB_MUSB_HDRC_HCD=y CONFIG_USB_GADGET_SELECTED=y # CONFIG_USB_GADGET_FUSB300 is not set # CONFIG_USB_GADGET_OMAP is not set # CONFIG_USB_GADGET_R8A66597 is not set # CONFIG_USB_GADGET_PXA_U2O is not set # CONFIG_USB_GADGET_M66592 is not set # CONFIG_USB_GADGET_DUMMY_HCD is not set I will try to find where the issues started to happen, but it's a bit difficult because all this USB Kconfig stuff is completely messed up. *Sigh* patches are welcome The were not last times: http://mid.gmane.org/1288656853-4625-1-git-send-email-felipe.contre...@gmail.com http://mid.gmane.org/1287482608-11320-1-git-send-email-felipe.contre...@gmail.com But I'll try again... Once I'm done with this endless bisect =/ Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] mfd: twl4030-irq: set irq nested flag
On Thu, Jun 30, 2011 at 12:51 PM, Felipe Balbi ba...@ti.com wrote:
threads from twl4030's children will be called
nested in the context of the demultiplexing
handler on twl4030-irq.c.
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/mfd/twl4030-irq.c | 11 ++-
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 1b9ab2f..ff16e9c 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -304,7 +304,7 @@ static irqreturn_t handle_twl4030_pih(int irq, void
*devid)
pih_isr;
pih_isr = 1, module_irq++) {
if (pih_isr 0x1)
- generic_handle_irq(module_irq);
+ handle_nested_irq(module_irq);
But handle_nested_irq calls action-thread_fn, which is NULL for the
sih handlers that are set-up below; handle_twl4030_sih will never be
called.
}
return IRQ_HANDLED;
@@ -596,9 +596,7 @@ static void handle_twl4030_sih(unsigned irq, struct
irq_desc *desc)
int isr;
/* reading ISR acks the IRQs, using clear-on-read mode */
- local_irq_enable();
isr = sih_read_isr(sih);
- local_irq_disable();
But handle_twl4030_sih is still not nested.
Fortunately, Neil Brown already fixed the issues in this patch by
making handle_twl4030_sih truly nested:
http://article.gmane.org/gmane.linux.kernel/1220708
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Interrupt issue in twl4030_keypad
Hi, The short version is this: either we revert this patch[1], or we apply this patch series[2], as well as its essential fixes[3]. The long version is this. There's a synchronization issue with the current keypad driver and twl core; the irq is marked as handled even though the thread that is supposed to handle it hasn't run yet, and since it's clear-on-read, and it hasn't been read, it's detected again, so they keypad driver receives two interrupt callbacks instead of one, and in the second one reads nothing from the i2c register, so a key release is assumed. This makes key-presses as simple as shift+a impossible. In other words, it's totally unreliable. This might not be isolated to the keypard driver, but other nested interrupts from twl core that started using request_threaded_irq prematurely (before it was supported by the twl core). But at least I haven't tried those. This patch was applied on 2.6.33, which means all versions before 3.2 are affected, including 3.1. What do you think about fixing this on stable kernels? Cheers. [1] http://article.gmane.org/gmane.linux.kernel/932255 [2] http://article.gmane.org/gmane.linux.ports.arm.omap/59958 [3] http://article.gmane.org/gmane.linux.ports.arm.omap/67333 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Interrupt issue in twl4030_keypad
On Mon, Dec 12, 2011 at 9:12 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Dec 12, 2011 at 08:30:49PM +0200, Felipe Contreras wrote: The short version is this: either we revert this patch[1], or we apply this patch series[2], as well as its essential fixes[3]. The long version is this. There's a synchronization issue with the current keypad driver and twl core; the irq is marked as handled even though the thread that is supposed to handle it hasn't run yet, and since it's clear-on-read, and it hasn't been read, it's detected again, so they keypad driver receives two interrupt callbacks instead of one, and in the second one reads nothing from the i2c register, so a key release is assumed. This makes key-presses as simple as shift+a impossible. In other words, it's totally unreliable. This might not be isolated to the keypard driver, but other nested interrupts from twl core that started using request_threaded_irq prematurely (before it was supported by the twl core). But at least I haven't tried those. This patch was applied on 2.6.33, which means all versions before 3.2 are affected, including 3.1. What do you think about fixing this on stable kernels? I believe Samuel has already applied those to the MFD tree. Yeap, I think Linus' tree is going to be fine (3.2), but I'm trying to find out what to do with previous kernels: as in, what should be done for stable trees. The funny part is that those patches were pending on linux-omap for over 2 months I have refreshed them over and over again and asked for help from other people to test. You mean these patches? [1] There's nothing wrong with pushing them forward, but I'm actually talking about this one [2]. Everything went smooth on my simple beagle board with no keypad and I couldn't see any issues, unfortunately. That's because you didn't get a single interrupt. You could have added a BUG() on handle_twl4030_pih() and you still wouldn't have noticed anything. Still, 2 months is a whole lot of time to NAK a patch, but nobody said anything so, of course, It's actually almost 2 years :) Samuel assumed it was ok and, like I said above, it worked for my simple GPIO usecase with beagleboard. Well, for 3.2 I think the situation is fine, but that's not what I'm talking about. About your GPIO tests, are you sure you got the interrupts through twl4030 irq handling? Cheers. [1] http://article.gmane.org/gmane.linux.ports.arm.omap/59958 [2] http://article.gmane.org/gmane.linux.kernel/932255 -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Interrupt issue in twl4030_keypad
On Mon, Dec 12, 2011 at 9:23 PM, Greg KH gre...@suse.de wrote: On Mon, Dec 12, 2011 at 08:30:49PM +0200, Felipe Contreras wrote: The short version is this: either we revert this patch[1], or we apply this patch series[2], as well as its essential fixes[3]. The long version is this. There's a synchronization issue with the current keypad driver and twl core; the irq is marked as handled even though the thread that is supposed to handle it hasn't run yet, and since it's clear-on-read, and it hasn't been read, it's detected again, so they keypad driver receives two interrupt callbacks instead of one, and in the second one reads nothing from the i2c register, so a key release is assumed. This makes key-presses as simple as shift+a impossible. In other words, it's totally unreliable. This might not be isolated to the keypard driver, but other nested interrupts from twl core that started using request_threaded_irq prematurely (before it was supported by the twl core). But at least I haven't tried those. This patch was applied on 2.6.33, which means all versions before 3.2 are affected, including 3.1. What do you think about fixing this on stable kernels? I think you need to tell me exactly what git commit ids in Linus's tree that you want to see in the stable kernel releases, which you didn't do here :( All right, I guess that rules out the revert option. Once Linus merges the latest fixes I can send you the commit ids, and they will work for 3.1, but I'm not sure for all the other versions since 2.6.33. The surest thing would be to just revert the patch I mentioned, which is a relatively small change compared to picking all those commits. Also, please use sta...@vger.kernel.org, sta...@kernel.org has been dead since August. Ok. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
