Re: [PATCH v2 00/15] tidspbridge driver MMU-related cleanups
Hi Laurent, On Wed, Sep 19, 2012 at 7:06 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hello, Here's the second version of my tidspbridge MMU-related cleanup patches. The first version has been sent privately only, don't try to search the mailing list archive for it :-) Replacing hw/hw_mmu.c and part of core/tiomap3430.c with generic IOMMU calls should be less difficult now. Anyone would like to give it a try? Laurent Pinchart (14): tidspbridge: hw_mmu: Reorder functions to avoid forward declarations tidspbridge: hw_mmu: Removed unused functions tidspbridge: tiomap3430: Reorder functions to avoid forward declarations tidspbridge: tiomap3430: Remove unneeded dev_context local variables tidspbridge: tiomap3430: Factor out common page release code tidspbridge: tiomap3430: Remove ul_ prefix tidspbridge: tiomap3430: Remove unneeded local variables tidspbridge: Fix VM_PFNMAP mapping tidspbridge: Remove unused hw_mmu_map_attrs_t::donotlockmpupage field arm: omap: iommu: Include required headers in iommu.h and iopgtable.h tidspbridge: Use constants defined in IOMMU platform headers tidspbridge: Simplify pte_update and mem_map_vmalloc functions tidspbridge: Use correct types to describe physical, MPU, DSP addresses tidspbridge: Replace hw_mmu_map_attrs_t structure with a prot bitfield Thanks, tested on beagle-xM, they look good! Can you submit them to Greg KH and de...@driverdev.osuosl.org, preferably with a 'staging:' prefix along with the current subject. The only thing of concern is that: ARM: OMAP: iommu: fix including iommu.h without IOMMU_API selected Might be taking a different path than these to mainline[1]. Cheers, Omar --- [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg76319.html -- 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 4/8] ARM: OMAP2+: hwmod: revise hardreset behavior
Hi Paul, On Thu, Apr 19, 2012 at 2:46 PM, Paul Walmsley p...@pwsan.com wrote: Hi Benoît, On Thu, 19 Apr 2012, Cousson, Benoit wrote: On 4/19/2012 7:17 PM, Paul Walmsley wrote: On Thu, 19 Apr 2012, Cousson, Benoit wrote: The concern of the people doing SW in these embedded processors was mainly because we were releasing the reset of processor without loading any SW first and thus the processor was executing some random instructions. So if we consider a better hwmods definition, we can potentially fix the MMU case: 'mmu_ipu': 'rst_ipu_mmu_cache' 'mmu_dsp': 'rst_dsp_mmu_cache' 'iva_config': 'rst_logic' Wouldn't these still be pseudo-hwmods? Or do each of these actually have address space, interconnect ports, etc.? Yes, these ones are the only real IPs for MPU stand point, with interconnect port and configuration registers. These are the MMUs documented in Chapter 20 of the OMAP4430 TRM vX, right? I don't really understand the interaction of the hardreset lines with these IPs. Chapter 20 doesn't seem to mention the PRCM-controllable hardreset lines at all. It only mentions the OCP_SYSCONFIG registers associated with them. Meanwhile Table 3-375 RM_DSP_RSTCTRL mentions a DSP MMU, cache, and slave interface reset line. Is that referring to the same MMU that is mentioned in Chapter 20? The end of Section 20.2 MMU Integration mentions two different DSP MMUs, a shared cache MMU and an L2 MMU - maybe the hardreset line only pertains to the first MMU? I don't thinks so, because trying to read the L2 MMU registers with DSP RST2 asserted causes an L3 error. DSP's shared cache MMU refers to section 5.3.2.5 Attribute MMU which is inside the dsp megamodule, OTOH L2 MMU refers to section 5.3.4 MMU, the latter is the one being referenced in section 20. But then we do have the processor resets that have to be exposed somewhere. 'ipu': 'rst_cpu0' 'rst_cpu1' 'dsp': 'rst_dsp' 'iva': 'rst_seq1' 'rst_seq2' None of these one should be controlled automatically. Don't we still want to put these IP blocks into reset until a driver for these IP blocks is loaded? Yes, indeed, my point is where are we going to declare these reset lines if we do not have any fake hwmods to store them. Wouldn't we associate them with the processor hwmods? e.g., omap44xx_iva_hwmod, omap44xx_dsp_hwmod ? Well, for the MMU we can, these are regular IPs (almost). The real issues was for the processors. Omar, do you know how these hardreset lines interact with the MMUs described in the TRM Chapter 20? For M3, RM_MPU_M3_RSTCTRL[2].RST3 releases both the cache and mmu interfaces, this one is needed if you want to program the mmu by touching any of the MMU registers, typical initialization is to deassert this line and then enable the M3 clock (since it is shared with its MMU) which should allow the reset to complete *only* after the clock has been enabled. For DSP, RM_DSP_RSTCTRL[1].RST2 it also releases dsp, cache and mmu (however the dsp doesn't boot until RST1 is deasserted), it is the same rationale as above; you deassert RST2, enable dsp clock (so reset can complete) and then program mmu. Touching any mmu registers without deasserting its reset line (even if the associated clock is ON) generates a L3 error. This can be found in 3.5.6 Reset sequences for DSP and M3. Regards, Omar -- 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 v3 4/4] ARM: omap: pass minimal SoC/board data for UART from dt
On Wed, Apr 11, 2012 at 3:39 AM, Cousson, Benoit b-cous...@ti.com wrote: Yes, it is a known issue and the fix was unfortunately was not merged during -rc phases but is fixed in 3.4 and should be fixed in 3.3 stable branch as well. I received the notification from Greg KH 2 weeks ago about the stable: Patch tty: serial: OMAP: Fix oops due to NULL pdata in DT boot has been added to the 3.3-stable tree You should just switch to 3.4-rc2 or get the patch if you are stuck to 3.3. Thanks, switching to 3.4-rc kernels fixed the problem (for cramfs), and right now this works for me. Regards, Omar -- 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 v3 4/4] ARM: omap: pass minimal SoC/board data for UART from dt
Hi,
On Wed, Dec 14, 2011 at 5:55 AM, Rajendra Nayak rna...@ti.com wrote:
Pass minimal data needed for console boot, from dt, for
OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
static initialization from generic board file.
...
diff --git a/arch/arm/mach-omap2/board-generic.c
b/arch/arm/mach-omap2/board-generic.c
index 63b5416..a508ed5 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -69,7 +69,6 @@ static void __init omap_generic_init(void)
if (node)
irq_domain_add_simple(node, 0);
- omap_serial_init();
omap_sdrc_init(NULL, NULL);
of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
--
1.7.1
I'm fairly new to DT and I'm trying to boot it with pandaboard and
k3.3, however above hunk deletes omap serial initialization, which
causes a panic on boot, because pdata is NULL:
static void serial_omap_start_tx(struct uart_port *port)
{
...
if (pdata-set_noidle)
Perhaps because this change skips the following path:
omap_serial_init-omap_serial_board_init-omap_serial_init_port
Where pdata is built in omap_device_build.
I'm just trying to confirm that I'm not alone or doing some silly
thing before getting in depth with the code.
Here it is the panic:
[2.024688] Unable to handle kernel NULL pointer dereference at
virtual address 0028
[2.031005] pgd = ed6f
[2.036315] [0028] *pgd=af339831, *pte=, *ppte=
[2.042907] Internal error: Oops: 17 [#1] SMP
[2.046630] Modules linked in:
[2.046630] CPU: 0Not tainted (3.3.0-2-gda19af1-dirty #11)
[2.046630] PC is at serial_omap_start_tx+0x1cc/0x218
[2.046630] LR is at serial_omap_start_tx+0x1b8/0x218
[2.067840] pc : [c02b3a54]lr : [c02b3a40]psr: 6193
[2.067840] sp : c21cbe70 ip : 0001 fp : a113
[2.073699] r10: ef107000 r9 : ed684600 r8 : 001c
[2.085327] r7 : 0002 r6 : 0007 r5 : r4 : ed684600
[2.086029] r3 : ef1281c0 r2 : fa02 r1 : 0004 r0 : c02b3a40
[2.086029] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM
Segment user
[2.101684] Control: 10c53c7d Table: ad6f004a DAC: 0015
[2.101684] Process S10udev (pid: 497, stack limit = 0xc21ca2f8)
[2.116973] Stack: (0xc21cbe70 to 0xc21cc000)
[2.123443] be60: a113
c0475b74 0002
[2.126007] be80: c02ac7e0 ef107000 ed684600 2113
c02ac830 ed690ad0
[2.139099] bea0: ed6e981c c02ae348 c02ae288 ef107000 c21ca000
001c ed6e9800 ef1074fc
[2.145385] bec0: c049cab4 ef1dcd80 ed6e9800 c0296aa8 6113
c02925e8 ef107194 ef107120
[2.152191] bee0: ef0003c0 ef1281c0 c0076214 ef1071b4
ef1071b4 ef1074b4 ef1dcd80
[2.159790] bf00: ef107000 b6f5c000 001c c21ca000 0400
c02968c8 c0292638
[2.159790] bf20: c06e3ea0 001c c02968c8 ef168b80 c21cbf80
ef1281c0 ef1dcd80
[2.175811] bf40: 001c b6f5c000 c21cbf80
c010841c
[2.188476] bf60: c0014400 ef1281c0 ef1dcd80 b6f5c000 001c
0004 c0108570
[2.195068] bf80: 001c 001c
b6f345c8 001c c00144a8
[2.195068] bfa0: c21ca000 c00142e0 001c b6f345c8 0001
b6f5c000 001c
[2.210723] bfc0: 001c b6f345c8 001c 0004 b6f5c000
0001
[2.210723] bfe0: be988948 b6e5ae98 b6eb7adc 6110
0001 89389af9 fef855e7
[2.226379] [c02b3a54] (serial_omap_start_tx+0x1cc/0x218) from
[c02ac830] (uart_start+0x68/0x6c)
[2.241973] [c02ac830] (uart_start+0x68/0x6c) from [c02ae348]
(uart_write+0xc0/0xe4)
[2.241973] [c02ae348] (uart_write+0xc0/0xe4) from [c0296aa8]
(n_tty_write+0x1e0/0x42c)
[2.257629] [c0296aa8] (n_tty_write+0x1e0/0x42c) from
[c0292638] (tty_write+0x140/0x23c)
[2.270446] [c0292638] (tty_write+0x140/0x23c) from [c010841c]
(vfs_write+0xb0/0x134)
[2.278594] [c010841c] (vfs_write+0xb0/0x134) from [c0108570]
(sys_write+0x40/0x70)
[2.281311] [c0108570] (sys_write+0x40/0x70) from [c00142e0]
(ret_fast_syscall+0x0/0x3c)
Regards,
Omar
--
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 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset
On Thu, Mar 15, 2012 at 1:25 AM, Paul Walmsley p...@pwsan.com wrote: Hola Omar, Hola :) On Wed, 14 Mar 2012, Ramirez Luna, Omar wrote: If we reached here the reset lines will be asserted, and then the code below touches sysc registers on a module under reset. Do you know of any case where this would be a problem? Seems like it would only affect IP blocks that have both hard reset lines and SYSCONFIG registers, and as far as I know, we don't have any of those defined? MMU (not yet mainlined) has both, a reset line and a sysconfig. I have been holding the hwmod for some time, but now that rpmsg/remoteproc is going to mainline it could make use of it along with omap3isp, however now I need to define functions to handle the reset lines (although I was fine with hwmod handling it). AFAIKnew, hwmod handling the reset line was fine (IMHO), the only 2 things were: - For the drivers to somehow make use of shutdown/enable if they needed they hwmod under reset and out. - The annoying: hwmod XX failed to hardreset because of the wrong reset sequence but causing no functional issues. Regards, Omar -- 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 5/8] ARM: OMAP2+: hwmod: ensure that SYSCONFIG bits are reprogrammed after a reset
Hi Paul,
2012/2/27 Paul Walmsley p...@pwsan.com:
diff --git a/arch/arm/mach-omap2/omap_hwmod.c
b/arch/arm/mach-omap2/omap_hwmod.c
index db4ad41..aeb6f4c 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1490,13 +1490,22 @@ static int _reset(struct omap_hwmod *oh)
pr_debug(omap_hwmod: %s: resetting\n, oh-name);
if (oh-class-reset)
- return oh-class-reset(oh);
-
- if (!oh-rst_lines_cnt)
- return _ocp_softreset(oh);
+ oh-class-reset(oh);
+ else if (!oh-rst_lines_cnt)
+ _ocp_softreset(oh);
+ else
+ for (i = 0; i oh-rst_lines_cnt; i++)
+ _assert_hardreset(oh, oh-rst_lines[i].name);
If we reached here the reset lines will be asserted, and then the code
below touches sysc registers on a module under reset.
This would crash on _setup-_setup_reset-_reset.
Adding a 'return 0' I believe fixes the behavior, boots the board and
leaves the hwmod under reset.
- else
+ } else {
for (i = 0; i oh-rst_lines_cnt; i++)
_assert_hardreset(oh, oh-rst_lines[i].name);
+ return 0;
+ }
- for (i = 0; i oh-rst_lines_cnt; i++)
- _assert_hardreset(oh, oh-rst_lines[i].name);
+ /*
+ * OCP_SYSCONFIG bits need to be reprogrammed after a
+ * softreset. The _enable() function should be split to avoid
+ * the rewrite of the OCP_SYSCONFIG register.
+ */
+ if (oh-class-sysc) {
+ _update_sysc_cache(oh);
+ _enable_sysc(oh);
+ }
Best Regards,
Omar
--
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: OMAP2: fix mailbox init code
Hi,
On Thu, Feb 23, 2012 at 3:18 AM, Bedia, Vaibhav vaibhav.be...@ti.com wrote:
On Thu, Feb 23, 2012 at 14:23:35, Ohad Ben-Cohen wrote:
[...]
Which happens on CONFIG_ARCH_OMAP2 !CONFIG_SOC_OMAP2420, due to
missing omap2_mboxes declaration.
[...]
-struct omap_mbox *omap2_mboxes[] = { mbox_dsp_info, mbox_iva_info, NULL };
+#ifdef CONFIG_ARCH_OMAP2
+struct omap_mbox *omap2_mboxes[] = {
+ mbox_dsp_info,
+#ifdef CONFIG_SOC_OMAP2420
+ mbox_iva_info,
+#endif
+ NULL
+};
#endif
#if defined(CONFIG_ARCH_OMAP4)
Instead of adding more #ifs can they be completely removed please?
I'll rebase/repost this series:
[PATCH 0/7] OMAP: mailbox: removing static declarations
http://comments.gmane.org/gmane.linux.ports.arm.omap/59620
In the meantime I would appreciate comments.
Regards,
Omar
--
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: Latest OMAP randconfig build error
On Wed, Feb 22, 2012 at 11:58 AM, Tony Lindgren t...@atomide.com wrote:
* Ohad Ben-Cohen o...@wizery.com [120222 01:30]:
+ Tony, Suman
On Wed, Feb 22, 2012 at 10:51 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
arch/arm/mach-omap2/mailbox.c: In function 'omap2_mbox_probe':
arch/arm/mach-omap2/mailbox.c:354: error: 'omap2_mboxes' undeclared (first
use in this function)
arch/arm/mach-omap2/mailbox.c:354: error: (Each undeclared identifier is
reported only once
arch/arm/mach-omap2/mailbox.c:354: error: for each function it appears in.)
The below should trivially solve this, but I wonder if there was any
other merit in explicitly using CONFIG_SOC_OMAP2420 there (any
different between 2420 and 2430 in that respect ?).
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 609ea2d..e61d275 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -258,7 +258,7 @@ struct omap_mbox mbox_dsp_info = {
struct omap_mbox *omap3_mboxes[] = { mbox_dsp_info, NULL };
#endif
-#if defined(CONFIG_SOC_OMAP2420)
+#if defined(CONFIG_ARCH_OMAP2)
/* IVA */
static struct omap_mbox2_priv omap2_mbox_iva_priv = {
.tx_fifo = {
2430 is like omap3 for the mailbox. So the code we have seems
wrong trying to initialize it like 2420 mailbox. So we either
need a new entry for omap2430_mboxes[], or should just bail
out from the probe for 2430 for the fix.
Yes, current code tries to configure both mboxes in a 2430, however it
shouldn't be assigning an irq line for the iva mbox, and any request
for iva mbox should fail due to that.
Code is wrong to register both in 2430 though.
Regards,
Omar
--
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 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. 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. Tidspbridge does know how to treat other clock request from dsp (gpt, mcbsp), it would be a matter of adding the logic in the arm side for any dsp image that is able to do it, however this doesn't apply to the current (latest) base image since it assumes the wdt3 is always controlled by tidspbridge. Regards, Omar -- 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 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. 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. Regards, Omar -- 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 9:18 PM, 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... Just FTR, the warning comes from an interconnect driver that reports errors. This specific warning occurs because the dsp tries to access wdt3 registers without a clock, hence turning ON the wdt3 (or default y for wdt3 Kconfig) will make the warning to disappear when first loading a base image into the dsp, because now there will be a clock for the registers. It depends on the firmware because the accesses are made from the dsp itself. Regards, Omar -- 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
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. Regards, Omar -- 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 2:11 AM, Felipe Contreras felipe.contre...@gmail.com wrote: 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. Indeed, the ideal would be the ability to turn off the config option but on the dsp side (when compiling the baseimage), however the release package only contains binaries given that the source is not open. I have been exploring introducing a flag in the shared memory area which would require a new baseimage release. Regards, Omar -- 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 2:21 AM, Dan Carpenter dan.carpen...@oracle.com wrote: On Mon, Jan 30, 2012 at 07:20:18PM -0600, Omar Ramirez Luna wrote: This structure is still used after it has been freed, since it is being allocated in probe, calls to free it have been moved to module's remove routine. This should fix the follwoing messages when attempting to remove the module: drv_get_first_dev_extension: Failed to retrieve the object handle drv_get_first_dev_extension: Failed to retrieve the object handle drv_destroy: Failed to store DRV object mgr_destroy: Failed to store MGR object So this is only triggered when you do an rmmod to remove the module? Yes. Probably that's not stable material. The critical issue is that for a small window the freed memory can be filled with something else and the driver still might dereference that memory which no longer belongs to it, thus causing a crash. But I guess that falls into this can be a problem, it is ok if it is being left out of stable. Thanks, Omar -- 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/24 Felipe Contreras felipe.contre...@gmail.com: 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. FWIW, I agree. Regards, Omar -- 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/24 Felipe Contreras felipe.contre...@gmail.com: 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. ... But looks good to me either way. FWIW, I agree. Regards, Omar -- 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 3/5] staging: tidspbridge: Lindent to drv_interface.c
2012/1/23 Víctor Manuel Jáquez Leal vjaq...@igalia.com:
No functional changes.
According to Lindent, the file drv_internface.c had some lines with bad
indentation.
This commit is the output of Lindent.
Usually lindent tends to do whatever it wants, unless carefully configured...
...
@@ -342,9 +342,10 @@ static void bridge_recover(struct work_struct *work)
if (atomic_read(bridge_cref)) {
INIT_COMPLETION(bridge_comp);
while (!wait_for_completion_timeout(bridge_comp,
-
msecs_to_jiffies(REC_TIMEOUT)))
- pr_info(%s:%d handle(s) still opened\n,
- __func__, atomic_read(bridge_cref));
+ msecs_to_jiffies
+ (REC_TIMEOUT)))
Like here, it just split msecs_to_jiffies(REC_TIMEOUT) into 2 lines
making it a little harder to read.
+ pr_info(%s:%d handle(s) still opened\n, __func__,
+ atomic_read(bridge_cref));
I remember the rule was to break lines as far to the right as
possible, no? Chapter 2 CodingStyle, same for the other similar
changes.
...
@@ -547,10 +548,9 @@ static int __devexit omap34_xx_bridge_remove(struct
platform_device *pdev)
pr_err(%s: Failed to retrieve the object handle\n, __func__);
goto func_cont;
}
-
Blank line removed?
#ifdef CONFIG_TIDSPBRIDGE_DVFS
if (cpufreq_unregister_notifier(iva_clk_notifier,
- CPUFREQ_TRANSITION_NOTIFIER))
+ CPUFREQ_TRANSITION_NOTIFIER))
pr_err(%s: cpufreq_unregister_notifier failed for iva2_ck\n,
__func__);
#endif /* #ifdef CONFIG_TIDSPBRIDGE_DVFS */
--
1.7.8.3
Regards,
Omar
--
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 4/5] staging: tidspbridge: silence the compiler
2012/1/23 Víctor Manuel Jáquez Leal vjaq...@igalia.com: Silence the warning when compiling drv_interface.c Signed-off-by: Víctor Manuel Jáquez Leal vjaq...@igalia.com --- drivers/staging/tidspbridge/rmgr/drv_interface.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c b/drivers/staging/tidspbridge/rmgr/drv_interface.c index c7015f5..3bbe443 100644 --- a/drivers/staging/tidspbridge/rmgr/drv_interface.c +++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c @@ -273,8 +273,9 @@ static int bridge_mmap(struct file *filp, struct vm_area_struct *vma) vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot); dev_dbg(bridge, %s: vm filp %p offset %x start %lx end %lx page_prot - %lx flags %lx\n, __func__, filp, offset, - vma-vm_start, vma-vm_end, vma-vm_page_prot, vma-vm_flags); + %ulx flags %lx\n, __func__, filp, offset, + vma-vm_start, vma-vm_end, vma-vm_page_prot, + vma-vm_flags); status = remap_pfn_range(vma, vma-vm_start, vma-vm_pgoff, vma-vm_end - vma-vm_start, -- 1.7.8.3 Looks good. Regards, Omar -- 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 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. Regards, Omar -- 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: 3.3-rc1 console lag (was: Re: [PATCH v8 00/20] OMAP2+: UART: Runtime adaptation + cleanup)
Hi Paul, On Wed, Jan 25, 2012 at 9:00 PM, Paul Walmsley p...@pwsan.com wrote: ... Ensure CONFIG_OMAP_PRM is set while testing irq_chaining with uart. And for pm_qos usage ensure CONFIG_CPU_IDLE is selected other wise console might be sluggish. There is console lag for omap2plus_defconfig given that CONFIG_CPU_IDLE is not enabled. Is the intention to force CPU_IDLE into the defconfig or find an alternative for the new pm_qos when cpu idle is disabled. Seen on beagle-xm and 3.3-rc1. Try this http://marc.info/?l=linux-arm-kernelm=132754676814391w=2 I tried the series and the console returned to normal, I can confirm that the following patch helps: tty: serial: OMAP: use a 1-byte RX FIFO threshold in PIO mode Thanks, Omar -- 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
3.3-rc1 console lag (was: Re: [PATCH v8 00/20] OMAP2+: UART: Runtime adaptation + cleanup)
Hi, On Fri, Nov 11, 2011 at 3:57 AM, Govindraj.R govindraj.r...@ti.com wrote: Converting uart driver to adapt to pm runtime API's. Code re-org + cleanup. Moving some functionality from serial.c to omap-serial.c ... Ensure CONFIG_OMAP_PRM is set while testing irq_chaining with uart. And for pm_qos usage ensure CONFIG_CPU_IDLE is selected other wise console might be sluggish. There is console lag for omap2plus_defconfig given that CONFIG_CPU_IDLE is not enabled. Is the intention to force CPU_IDLE into the defconfig or find an alternative for the new pm_qos when cpu idle is disabled. Seen on beagle-xm and 3.3-rc1. Regards, Omar -- 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: Bad page state in tidspbridge driver
Hi Laurent, On Sun, Jan 8, 2012 at 12:17 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi, I'm hitting what seems to be a tidspbridge driver issue when using gst-dsp with the DSP JPEG encoder. My application captures images from the OMAP3 ISP directly to frame buffer memory for display. It then passes the frame buffer buffer pointer to gst-dsp and the tidspbridge driver, which maps them to the DSP IOMMU address space. Frame buffer sounds like kernel memory not user's, this because tidspbridge map functions do get_user_pages on the memory passed to it, unless you set VM_IO (I believe, so tidspbridge just does a get_page), however the issue might be related with the page count in the end. When starting the JPEG encoder a bunch of identical messages are printed to the kernel log: [14643.065490] BUG: Bad page state in process source:src pfn:89e39 [14643.072143] page:c0b11720 count:0 mapcount:0 mapping: (null) index:0x0 [14643.079437] page flags: 0x410(dirty|reserved) [14643.084197] [c003ee5c] (unwind_backtrace+0x0/0xe0) from [c00a4c34] (bad_page+0xc4/0xec) [14643.093505] [c00a4c34] (bad_page+0xc4/0xec) from [c00a5480] (free_pages_prepare+0x70/0xe8) [14643.102935] [c00a5480] (free_pages_prepare+0x70/0xe8) from [c00a5634] (free_hot_cold_page+0x20/0x1ac) [14643.113525] [c00a5634] (free_hot_cold_page+0x20/0x1ac) from [bf00d120] (bridge_brd_mem_un_map+0x12c/0x3d4 [bridgedriver]) [14643.126007] [bf00d120] (bridge_brd_mem_un_map+0x12c/0x3d4 [bridgedriver]) from [bf01cda8] (proc_un_map+0x6c/0xa0 [bridgedriver]) [14643.139404] [bf01cda8] (proc_un_map+0x6c/0xa0 [bridgedriver]) from [bf014490] (api_call_dev_ioctl+0xe8/0x10c [bridgedriver]) [14643.152160] [bf014490] (api_call_dev_ioctl+0xe8/0x10c [bridgedriver]) from [bf0208b8] (bridge_ioctl+0x12c/0x15c [bridgedriver]) [14643.165191] [bf0208b8] (bridge_ioctl+0x12c/0x15c [bridgedriver]) from [c00d8fd8] (do_vfs_ioctl+0x4e8/0x55c) [14643.176177] [c00d8fd8] (do_vfs_ioctl+0x4e8/0x55c) from [c00d9080] (sys_ioctl+0x34/0x54) [14643.185333] [c00d9080] (sys_ioctl+0x34/0x54) from [c003a700] (ret_fast_syscall+0x0/0x3c) The backtrace is printed because the page has the PG_reserved flag set, which is expected (as far as I know) for the frame buffer memory. Yes, PG_reserved is set, and given that tidspbridge is unmapping, it is also telling the kernel that the pages are not going to be used anymore, however in the beginning the memory was allocated by either frame buffer or display driver. I believe the issue is because the page count reaches to 0 but PG_reserved is set as you point out. Is this use case unsupported ? Not entirely supported, as long as the memory used by other drivers increases the page count of each page there shouldn't be any issue, since bridge doesn't know which pages have more than X number of users. Regards, Omar -- 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 Tue, Dec 27, 2011 at 3:41 AM, Felipe Contreras felipe.contre...@gmail.com wrote: 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. I'm sorry, I don't seem to follow... you want _enable in platform driver, and you call drivers/iommu/omap-iommu.c the platform driver... And that is precisely where pm_runtime_enable() is, no? Regards, Omar -- 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
Hi Laurent On Sun, Dec 25, 2011 at 3:08 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: 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. The device should be able to be put to sleep at anytime when it is NOT being used. AFAIK there is no mechanism for the main processor (the one running the kernel) to know when the other iommus are not being used, given that they are in independent processors/subsystems, at least for the ones in the DSP or M3 processors. Once the user releases its iommu resource it means it is no longer using it, at that point the device can be put to sleep. How should the OMAP3 ISP driver proceed to make sure that its IOMMU is clocked off when it doesn't need it ? If there is an specific scenario where the iommu should be disabled, iommu_detach_device can be called to release the iommu resource. On suspend/resume scenarios runtime pm callbacks should still be able to put the device in idle. Regards, Omar -- 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 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. Regards, Omar -- 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 Mon, Dec 19, 2011 at 10:11 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
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?
As sure as 4.7.4.1.7 CAM Power Domain ;), if someone else could
clarify would be great to avoid the are you sure discussion.
+/* 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.
The camera subsystem interface is clocked with the L3 and L4 clocks
(CAM_L3_ICLK and CAM_L4_ICLK, respectively). CAM_L3_ICLK is also used
as the main functional clock. The functional clock (CAM_MCLK) is
provided by DPLL4 to supply the external sensor.
Either CAM subsystem section or PRCM - CAM PWRDM is ambiguous or wrong.
...
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.
Would be good to clarify the intentions to keep the code as it is.
+ .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.
Yes, you should receive an interrupt if the clock is enabled and the
iommu is being used, hence the part inside in the ISR to enable the
clocks was removed.
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.
The device should be able to be put to sleep at anytime when it is NOT
being used. AFAIK there is no mechanism for the main processor (the
one running the kernel) to know when the other iommus are not being
used, given that they are in independent processors/subsystems, at
least for the ones in the DSP or M3 processors. Once the user releases
its iommu resource it means it is no longer using it, at that point
the device can be put to sleep.
Regards,
Omar
--
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 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. Regards, Omar -- 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:39 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:
+/* 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,
I believe in this case cam_ick is used as the main clock as it
supplies both functional and interface.
+/* 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.
+ .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.
Regards,
Omar
--
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:33 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: @@ -123,11 +123,10 @@ static int iommu_enable(struct omap_iommu *obj) if (!arch_iommu) return -ENODEV; - clk_enable(obj-clk); + pm_runtime_get_sync(obj-dev); err = arch_iommu-enable(obj); - clk_disable(obj-clk); return err; } Hmm, this is called on omap_iommu_attach, and iommu_disable is not called until omap_iommu_detach, so this means the device will never sleep. Why don't you call pm_runtime_put() instead of clk_disable()? Here once you call pm_runtime_put, the hwmod should turn off iva2_ck leaving device unpowered, not usable if it is part of an independent module and that module is still awake, but since the module clock feeds both the dsp and the mmu this doesn't occur. However might be possible if there is a specific clock to feed the mmu and another for the mmu user. In theory iommu should be independent on handling its clock resources, take tidspbridge, it powers iva2_clk which indirectly powers iva_mmu, so as long as the dsp is powered and using the mmu (omap_iommu_attach) the iommu should be ON, once you omap_iommu_detach means that the device is no longer using the iommu and can be put to sleep. This could be helpful if your main processor can go to sleep independently of other processors, say you attach the iommu and leave the dsp doing calculations and memory accesses through mmu while the main processor decides to enter sleep. All the rest of the calls to pm_runtime_get/put after this are basically no-ops, because the count will never go below 1. I didn't try but without calling iommu_attach the other functions that use pm_runtime_get/put can still be called as they are exported symbols. I believe omap-iommu-debug.c does something like this to dump stuff. This should be cleaned up/audited, IMHO, but it doesn't seem to belong to this conversion series. You mention some irq issues, but could it be due to some bad clocks in the hwmod data? There are no issues with irqs, just a weird case, see below... ... @@ -780,9 +777,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data) if (!obj-refcount) return IRQ_NONE; - clk_enable(obj-clk); errs = iommu_report_fault(obj, da); - clk_disable(obj-clk); Why? In order to get an irq from the mmu, the mmu should be functional and have a clock, but iommu_enable used to enable and disable the clock. In the end you are able to get an irq because someone else (tidspbridge) has the mmu indirectly powered (since they share the same clock), I felt this shouldn't be and the iommu should handle its own clocks even if they are shared with other modules. Hence iommu_enable does pm_runtime_get for the life time of the user of the mmu. ... @@ -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. No, the driver should resume only if there are outstanding wake up requests, right? I don't seem to get this question. Thanks, Omar -- 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: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. Regards, Omar -- 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: mcbsp: Fix possible memory corruption
On Mon, Dec 12, 2011 at 12:33 PM, Tony Lindgren t...@atomide.com wrote: Applying into fixes. FWIW, I was bisecting this issue, the patch fixes the following slab corruptions on boot log: ... hw-breakpoint: found 5 (+1 reserved) breakpoint and 1 watchpoint registers. hw-breakpoint: maximum watchpoint size is 4 bytes. Slab corruption: size-32 start=eea66740, len=32 010: d8 9d 02 c0 04 9d 02 c0 6b 6b 6b 6b 6b 6b 6b a5 kkk. Prev obj: start=eea66720, len=32 000: 6f 6d 61 70 32 5f 6d 63 73 70 69 2e 31 00 5a 5a omap2_mcspi.1.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Next obj: start=eea66760, len=32 000: 70 6f 77 65 72 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a power.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Slab corruption: size-32 start=eea66740, len=32 010: d8 9d 02 c0 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkk. Prev obj: start=eea66720, len=32 000: 6f 6d 61 70 32 5f 6d 63 73 70 69 2e 31 00 5a 5a omap2_mcspi.1.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Next obj: start=eea66760, len=32 000: 70 6f 77 65 72 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a power.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Slab corruption: size-32 start=eea66740, len=32 010: d8 9d 02 c0 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkk. Prev obj: start=eea66720, len=32 000: 6f 6d 61 70 32 5f 6d 63 73 70 69 2e 31 00 5a 5a omap2_mcspi.1.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Next obj: start=eea66760, len=32 000: 70 6f 77 65 72 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a power.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Slab corruption: size-32 start=eea66740, len=32 010: d8 9d 02 c0 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkk. Prev obj: start=eea66720, len=32 000: 6f 6d 61 70 32 5f 6d 63 73 70 69 2e 31 00 5a 5a omap2_mcspi.1.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. Next obj: start=eea66760, len=32 000: 70 6f 77 65 72 00 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a power.ZZ 010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a a5 ZZZ. OMAP DMA hardware revision 0.0 ... Regards, Omar -- 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: remove header files included more than once
Hi Haojian, On Sun, Dec 11, 2011 at 7:10 PM, Haojian Zhuang haojian.zhu...@gmail.com wrote: ... Hi Omar, Thanks for your patch. Could you help to split your patches into small pieces? Since different people are handling for different files. For example, I can help to handle files in arch-pxa. But I can't handle other files. No problem, I'll split the patch to be included by independent trees. Thanks, Omar -- 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] ARM: OMAP: dmtimer: fix sleeping function called from invalid context
On Mon, Dec 12, 2011 at 5:08 PM, Tony Lindgren t...@atomide.com wrote:
...
@@ -134,22 +134,13 @@ static void omap_dm_timer_reset(struct
omap_dm_timer *timer)
int omap_dm_timer_prepare(struct omap_dm_timer *timer)
{
...
- ret = omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
...
All clocks requested are set to 32 KHz first, even with the current
approach, there exists another API to set a new source.
To be honest I don't know why the clocks are set to 32 KHz first,
maybe the default call path for users should be:
You need a functional clock for the timer registers to work
I believe.
Sounds logic :)
omap_dm_timer_request
Yes this would make sense. The omap_timer_list should be protected
by a mutex. There should not be a need for spinlock there as
omap_dm_timer_request should be only called during init. If that's
not the case, the the driver using it is broken.
Ok, I made this patch thinking that 'request' could be called from any
context, but if that is not the case mutex should be fine.
omap_dm_timer_set_source (new explicit call)
omap_dm_timer_start
Once the timer has been requested, there should not be a need
for locking as there should be only one timer user using the
timer specific registers.
And remove setting the source to 32 KHz by default in omap_dm_timer_request.
That you may need to be able to do anything with the timer :)
Well the intention was for the user to call it explicitly so it didn't
look as a hard coded setting, but I can keep it.
IIUC, mutex should be held instead of spin lock, I can do the change
instead of this patch and see how it goes.
Thanks,
Omar
--
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] staging: tidspbridge: request dmtimer clocks on init
On Fri, Dec 9, 2011 at 4:38 PM, Greg KH g...@kroah.com wrote: How about for 3.2.1? I'll try to get this into 3.2, but I can't guarantee anything so late in the release cycle, sorry. Sounds fair, fingers crossed for 3.2 ;) Thanks, Omar -- 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] ARM: OMAP: dmtimer: fix sleeping function called from invalid context
On Fri, Dec 9, 2011 at 3:34 PM, Tony Lindgren t...@atomide.com wrote:
+ ret = omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
...
EXPORT_SYMBOL_GPL(omap_dm_timer_request);
This does not seem right.. It seems that you're hardcoding the source
clock to 32 KiHz clock while other sources are available too?
Agree, but... (below)
+ ret = omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
...
And here too?
Agree but that is the default behavior set by dm timer framework:
@@ -134,22 +134,13 @@ static void omap_dm_timer_reset(struct
omap_dm_timer *timer)
int omap_dm_timer_prepare(struct omap_dm_timer *timer)
{
...
- ret = omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
...
All clocks requested are set to 32 KHz first, even with the current
approach, there exists another API to set a new source.
To be honest I don't know why the clocks are set to 32 KHz first,
maybe the default call path for users should be:
omap_dm_timer_request
omap_dm_timer_set_source (new explicit call)
omap_dm_timer_start
And remove setting the source to 32 KHz by default in omap_dm_timer_request.
Regards,
Omar
--
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] staging: tidspbridge: request dmtimer clocks on init
On Wed, Dec 7, 2011 at 4:11 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Given that dm timer framework doesn't support request of clocks by soft | hard irqs because some recent changes, tidspbridge needs to request its clocks on init and enable/disable them on demand. This was first seen on 3.2-rc1. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com This looks similar similar to this one: http://article.gmane.org/gmane.linux.ports.arm.omap/61816 Are you sure this is what we want instead of that one? I believe your patch only takes care of gpt5 and gpt6, but not of the other 2 that could be requested by the dsp, let's say gpt8 for avsync. As for this patch, dm timer framework should support request on soft or hard irq[1], but it can't because the underlying functions to use clk_get-clk_get_sys which holds a mutex. I believe your error was seen because of patches not in mainline, but the concept between the patches is similar indeed, however mine: - Handles the other gpt that can be requested by the dsp. - Uses start/stop according to future plans of making enable/disable static. - Might be temporal if *dm_timer_request* functions are fixed and there is an overall feeling that we can revert to use request+start/stop+free on request. Regards, Omar, --- [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg58665.html -- 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] staging: tidspbridge: request dmtimer clocks on init
On Thu, Dec 8, 2011 at 1:50 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Dec 8, 2011 at 9:10 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Wed, Dec 7, 2011 at 4:11 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Given that dm timer framework doesn't support request of clocks by soft | hard irqs because some recent changes, tidspbridge needs to request its clocks on init and enable/disable them on demand. This was first seen on 3.2-rc1. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com This looks similar similar to this one: http://article.gmane.org/gmane.linux.ports.arm.omap/61816 Are you sure this is what we want instead of that one? I believe your patch only takes care of gpt5 and gpt6, but not of the other 2 that could be requested by the dsp, let's say gpt8 for avsync. I'm not really sure about that... Judging from the code, the only timers needed used to be initialized on bridge_brd_start based on the symbols of the baseimage. If you say that's not enough, then I guess that's fine (not really surprising that the old code missed that). I meant the dsp by itself can generate that request. That is why it is not explicitly seen in the code. - Handles the other gpt that can be requested by the dsp. And I guess which clocks are going to be used is not discoverable somehow... It is basically the pool of clocks of the dsp is formed by mcbsp, gpt, wdt, ssi. The dsp can request any gpt from 5 to 8 according to the binary you are running on the dsp side. That's the reason it is somewhat hidden what clock the dsp needs. - Uses start/stop according to future plans of making enable/disable static. Yeah, but that's easy to change s/enable/start/ s/disable/stop/ Agree - Might be temporal if *dm_timer_request* functions are fixed and there is an overall feeling that we can revert to use request+start/stop+free on request. At which point we would end up with something similar to my patch :) Yes, plus handling the other clocks that can be available to the dsp. Anyway, I guess there's not much problem in requesting extra clocks that we would not use. AV playback does use gpt8, but for systems that doesn't use gpt8 it is best to use the old approach of just request/free prior to dm timer changes. Regards, Omar -- 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] staging: tidspbridge: request dmtimer clocks on init
On Thu, Dec 8, 2011 at 2:11 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: Given that dm timer framework doesn't support request of clocks by soft | hard irqs because some recent changes, tidspbridge needs to request its clocks on init and enable/disable them on demand. This was first seen on 3.2-rc1. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com Tested-by: Felipe Contreras felipe.contre...@gmail.com Reviewed-by: Felipe Contreras felipe.contre...@gmail.com Thanks! Something the commit message didn't mention is that this fixes a nasty continuous loop that happens as soon as you load the module. Indeed. Definitely 3.2-fix material IMO :) I believe it was pushed to staging-next... I'm rooting so it can be pushed to staging-linus :) Regards, Omar -- 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] staging: tidspbridge: request dmtimer clocks on init
On Thu, Dec 8, 2011 at 6:07 PM, Greg KH g...@kroah.com wrote: Definitely 3.2-fix material IMO :) I believe it was pushed to staging-next... I'm rooting so it can be pushed to staging-linus :) As it wasn't originally stated that this was a problem that needed to be fixed for 3.2, can it wait for 3.3-rc1? Especially given that there is still discussion about this? No discussion from me either. The original intention was to include this patch for 3.2 because of the recent changes in dm timer framework, I apologize if This was first seen on 3.2-rc1. is too ambiguous. I hope this can make it for 3.2 though. Thanks, Omar -- 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: dmtimer: fix missing content/correction in low-power mode support
Hi,
On Tue, Nov 8, 2011 at 12:00 AM, Tarun Kanti DebBarma
tarun.ka...@ti.com wrote:
Since omap_dm_timer_write_reg/__omap_dm_timer_write is now modified
to use timer-func_base OCP_CFG should not use this wrapper anymore.
Instead use __raw_writel() directly and use timer-io_base instead
to write to OCP_CFG.
The timer-sys_stat is valid only if timer-revision is 1. In the
context restore function make this correction.
Save the contexts and loss count when timer is stopped.
Also, disable the clock. Else, clock usecount would become imbalanced.
Signed-off-by: Tarun Kanti DebBarma tarun.ka...@ti.com
...
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index af3b92b..f042c82 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
...
@@ -357,6 +357,21 @@ int omap_dm_timer_stop(struct omap_dm_timer *timer)
__omap_dm_timer_stop(timer, timer-posted, rate);
+ if (timer-loses_context) {
+ if (timer-get_context_loss_count)
Maybe: if (timer-loses_context timer-get_context_loss_count)
+ timer-ctx_loss_count =
+ timer-get_context_loss_count(timer-pdev-dev);
could get rid of this weird one-line formatting
+ }
+
+ /*
+ * Since the register values are computed and written within
+ * __omap_dm_timer_stop, we need to use read to retrieve the
+ * context.
+ */
+ timer-context.tclr =
+ omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
+ timer-context.tisr = __raw_readl(timer-irq_stat);
+ omap_dm_timer_disable(timer);
FWIW, functionally it looks good to me, besides it fixes the broken
dmtimer start/stop sequence from 3.2-rc1. Tested with tidspbridge on
omap3.
If needed:
Tested-by: Omar Ramirez Luna omar.rami...@ti.com
Regards,
Omar
--
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: request dmtimer clocks on init
Hi Greg, On Fri, Nov 18, 2011 at 3:54 PM, Omar Ramirez Luna omar.rami...@ti.com wrote: diff --git a/drivers/staging/tidspbridge/core/dsp-clock.c b/drivers/staging/tidspbridge/core/dsp-clock.c index 3d1279c..1ba10ae 100644 --- a/drivers/staging/tidspbridge/core/dsp-clock.c +++ b/drivers/staging/tidspbridge/core/dsp-clock.c @@ -54,6 +54,7 @@ /* Bridge GPT id (1 - 4), DM Timer id (5 - 8) */ #define DMT_ID(id) ((id) + 4) +#define DM_TIMER_CLOCKS 5 :/ typo, please ignore this patch there are only 4 clocks for the dsp. I'll send v2 instead. Thanks, Omar -- 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 v3 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp
Hi, On Fri, Nov 4, 2011 at 6:23 PM, Kevin Hilman khil...@ti.com wrote: + .flags = HWMOD_INIT_NO_RESET, Why is this needed? ... + .flags = HWMOD_INIT_NO_RESET, And this? I have this because the hwmod complains about a failure in hard reset, even though the reset deassert does complete after the clock is enabled. Later on, hwmod will warn again because of a wrong state when enabling, I believe because of the failure on _setup but didn't dig into it yet. Regards, Omar -- 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 v3 4/4] OMAP3/4: iommu: adapt to runtime pm
Hi, On Fri, Nov 4, 2011 at 6:27 PM, Kevin Hilman khil...@ti.com wrote: @@ -821,9 +820,7 @@ static irqreturn_t iommu_fault_handler(int irq, void *data) if (!obj-refcount) return IRQ_NONE; - clk_enable(obj-clk); errs = iommu_report_fault(obj, da); - clk_disable(obj-clk); if (errs == 0) return IRQ_HANDLED; I'm not terribly familiar with this IOMMU code, but this one looks suspiciou because you're removing the clock calls but not replacing them with runtime PM get/put calls. I just want to make sure that's intentional. If so, you might want to add a comment about that to the changelog. Yes it is intentional, reason is that in order to get an interrupt, the device should be powered on in advance, right now it is working because the modules share a common clock so the users of the omap-iommu indirectly give power to it. However I made another change to do pm_runtime_get/put on attach/detach so it doesn't rely on others to keep the clocks on. I'll add the comment. Thanks, Omar -- 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 v3 0/4] OMAP: iommu: hwmod support and runtime PM
+ Joerg, iommu-list On Tue, Nov 1, 2011 at 5:15 PM, Omar Ramirez Luna omar.rami...@ti.com wrote: Introduced hwmod support for OMAP3 (iva, isp) and OMAP4 (ipu, dsp), along with the corresponding runtime PM routines to deassert reset lines, enable/disable clocks and configure sysc registers. v3: - Rebased to 3.1-rc10 lo rebuilt, added structure terminators, and removed .omap_chip field. v2: - Added oh reset info to assert/deassert mmu reset lines. - Addressed previous comments on v1 http://www.spinics.net/lists/arm-kernel/msg103271.html Due to compatibility an ifdef needs to be propagated (previously on iommu resource info) to hwmod data in OMAP3, so users of iommu and tidspbridge can avoid issues of two modules managing mmu data/irqs/resets; this until tidspbridge can be safely migrated to iommu framework. Omar Ramirez Luna (4): OMAP3: hwmod data: add mmu data for iva and isp OMAP4: hwmod data: add mmu hwmod for ipu and dsp OMAP3/4: iommu: migrate to hwmod framework OMAP3/4: iommu: adapt to runtime pm arch/arm/mach-omap2/iommu2.c | 36 - arch/arm/mach-omap2/omap-iommu.c | 162 - arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 131 + arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 154 ++-- arch/arm/plat-omap/include/plat/iommu.h | 17 ++- arch/arm/plat-omap/include/plat/iommu2.h | 2 - drivers/iommu/omap-iommu.c | 49 +++ drivers/media/video/omap3isp/isp.c | 2 +- drivers/staging/tidspbridge/core/tiomap3430_pwr.c | 2 +- 9 files changed, 339 insertions(+), 216 deletions(-) -- 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 v3 4/4] OMAP3/4: iommu: adapt to runtime pm
Hi MyungJoo, On Wed, Nov 2, 2011 at 5:16 AM, MyungJoo Ham myungjoo@gmail.com wrote: On Wed, Nov 2, 2011 at 7:15 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. Tidspbridge uses a macro removed with this patch, for now the value is hardcoded to avoid breaking compilation. 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 | 46 - drivers/staging/tidspbridge/core/tiomap3430_pwr.c | 2 +- 6 files changed, 19 insertions(+), 51 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index bbbf747..3c55be0 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -123,11 +123,11 @@ static int iommu_enable(struct omap_iommu *obj) if (!arch_iommu) return -ENODEV; - clk_enable(obj-clk); + pm_runtime_enable(obj-dev); + pm_runtime_get_sync(obj-dev); err = arch_iommu-enable(obj); - clk_disable(obj-clk); return err; } @@ -136,11 +136,10 @@ static void iommu_disable(struct omap_iommu *obj) if (!obj) return; - clk_enable(obj-clk); - arch_iommu-disable(obj); - clk_disable(obj-clk); + pm_runtime_put_sync(obj-dev); + pm_runtime_disable(obj-dev); } I'm just curious here... Is there any reason to do pm_runtime_enable/disable at iommu_enable/iommu_disable which are called by iommu_attach/detach? I thought that normally, ideal locations of pm_runtime_enable/disable for such devices are in probe/remove() because it assures that the device is suspended after the probe. It seems that the device might be kept on after probe and before the first iommu_attach if it is default-on. The default state of these MMUs is on reset and needs to be deasserted to be used, but you're right, it makes more sense to move pm_runtime_enable/disable calls to probe/remove. I'll wait a bit, do this change and resubmit. Thanks for the comment, Omar -- 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 v3 4/4] OMAP3/4: iommu: adapt to runtime pm
On Wed, Nov 2, 2011 at 10:21 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Nov 01, 2011 at 05:15:52PM -0500, Omar Ramirez Luna wrote: Use runtime PM functionality interfaced with hwmod enable/idle functions, to replace direct clock operations, reset and sysconfig handling. Tidspbridge uses a macro removed with this patch, for now the value is hardcoded to avoid breaking compilation. You probably want to include people involved with power management on this, so maybe the linux-pm mailing list, and those involved with runtime-pm stuff (I think Rafael qualifies as the maintainer for this stuff, even if he's not listed in MAINTAINERS.) Will do, I'll submit it again if no other comments are received. Regards, Omar -- 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: tidspbridge issue with omap_dm_timer_free
Hi, On Sun, Aug 14, 2011 at 2:44 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Yeah, but with the current approach it would be possible that everything works fine, and then the DSP goes to hibernation, a module is loaded that uses one dm timer, and then when the DSP wakes up, that timer would fail, there isn't even a check for that. If I follow the code correctly, when the DSP goes back to hibernation, there would be a crash, as a NULL timer would be freed. I think that's too error prone. Yes, this scenario could trigger such error condition. So, FWIW, I'm fine with just a check or your approach. If possible to apply the same mechanism for the other clocks, so in the enable/disable functions all the clocks are then enabled/disabled avoiding mixed behavior of enable/request, disable/free between gpt and mcbsp clocks. Regards, Omar -- 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: tidspbridge issue with omap_dm_timer_free
On Wed, Aug 10, 2011 at 1:42 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
Hi,
I am trying to use a more recent version of the tidspbridge code in
the Nokia N9, but I'm stuck with this warning that is caused by using
the dm timer framework.
[ 30.883636] BUG: sleeping function called from invalid context at
kernel/mutex.c:287
[ 30.885925] in_atomic(): 1, irqs_disabled(): 0, pid: 305, name: mboxd/0
[ 30.892517] 3 locks held by mboxd/0/305:
[ 30.896392] #0: (mboxd){+.+...}, at: [b0081778]
worker_thread+0x154/0x2bc
[ 30.903533] #1: (mq-work){+.+...}, at: [b0081778]
worker_thread+0x154/0x2bc
[ 30.911041] #2: (pwr_lock){+.}, at: [af12f870]
handle_hibernation_from_dsp+0x1c/0x158 [bridgedriver]
[ 30.920959] [b003f75c] (unwind_backtrace+0x0/0xd4) from
[b03a3ac4] (mutex_lock_nested+0x30/0x32c)
[ 30.930175] [b03a3ac4] (mutex_lock_nested+0x30/0x32c) from
[b00568f8] (clk_set_parent+0x34/0xf8)
...
From what I can see this could be triggered in upstream by enabling PM
and debug mutex stuff right after loading the baseimage.
Any ideas?
May I know which kernel version are you using? I can't seem to find
the mutex_lock inside clk_set_parent in arch/arm/plat-omap/clock.c.
Regards,
Omar
--
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: tidspbridge issue with omap_dm_timer_free
On Wed, Aug 10, 2011 at 6:27 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, Aug 10, 2011 at 9:42 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I am trying to use a more recent version of the tidspbridge code in the Nokia N9, but I'm stuck with this warning that is caused by using the dm timer framework. Actually, the problem only happens on the 'dspbridge' branch, which has some unmerged patches. These patches introduce some new mutexes that trigger this. The only lock introduced is a spin_lock_bh which makes sense to warn if the underlying functions have a mutex, but I couldn't find the clk_set_parent mutex which is causing this on the omap files. My proposed solution is to request the dm timers at initialization time, and just enable/disable them on wake/hibernate, which is a bit similar to what was happening before, and probably more efficient and proper. When I made these changes I wanted to avoid keeping an array with the clocks requested and nobody else being able to use them (even if that meant a conflict between two drivers trying to use the same clock, but other warnings could help to point to such cases). Also I wanted to maintain uniformity, not just requesting some clock, freeing them and then some others to be held and freed only on module removal. That said, I have no problem on changing to your approach if needed. Regards, Omar I'm attaching the patch. -- 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/8] staging: tidspbridge: PM routines cleanup
On Wed, Mar 23, 2011 at 1:49 PM, Omar Ramirez Luna omar.rami...@ti.com wrote: Cleanup and fixes for tidspbridge's PM routines. The most significant ones are: - Protection of critical sections when disabling clocks which could cause issues during PM transitions. - Fix of resume path from RET power state. Other patches are cleanups and preparations for the usage of spinlock. Omar Ramirez Luna (8): staging: tidspbridge: make wake_dsp to handle PM code staging: tidspbridge: fix resume path from retention staging: tidspbridge: remove msleep for dsp transition wait staging: tidspbridge: send mbox PM command directly staging: tidspbridge: send wake message even if dsp is running staging: tidspbridge: remove redundant code from PM routines staging: tidspbridge: remove redundant indentation in PM routines staging: tidspbridge: protect critical sections on PM routines drivers/staging/tidspbridge/core/tiomap3430_pwr.c | 248 ++--- drivers/staging/tidspbridge/core/tiomap_io.c | 58 +- 2 files changed, 168 insertions(+), 138 deletions(-) Pushed to dspbridge. Regards, Omar -- 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] OMAP: iovmm: fix SW flags passed by user
Hi Hiroshi, On Fri, Mar 25, 2011 at 3:04 PM, Omar Ramirez Luna omar.rami...@ti.com wrote: Commit d038aee24dcd5a2a0d8547f5396f67ae9698ac8e omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED flag, changes iovmm to receive flags specified by user, however the upper 16 bits of the flags are wiped by iovmm itself. This fixes IOVMF_DA_FIXED flags from being lost, and lets the user map its desired device addresses. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- v2: Include missing reference (commit and name) to patch in description. If no comments, could you ack this patch? Regards, Omar -- 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: HELP:dspbridge:how to configure GPT timer
Hi, On Sat, Apr 9, 2011 at 7:38 AM, onlyfever onlyfe...@gmail.com wrote: Hi all! currently dsp make use of gpt 5, 6, 7 or 8. I'm using gpt 8 for something else. Is it possible to configure dsp-bridge to use other gpt timer? I want to change gpt 8 to gpt 9,or gpt 7. Please give me some advice. GPT8 is used for avsync on the dsp side, unfortunately there is no way to change this without dsp sources and the latter are not publicly available :/ Regards, Omar -- 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/5] OMAP: l3: fixes and cleanup
On Wed, Mar 30, 2011 at 1:45 AM, Santosh Shilimkar santosh.shilim...@ti.com wrote: OMAP3: l3: fix for irq 10: nobody cared message OMAP3: l3: fix omap3_l3_probe error path OMAP3: l3: minor cleanup for error message, parenthesis and extra lines OMAP4: l3: fix omap4_l3_probe error path OMAP4: l3: minor cleanup for parenthesis and extra spaces Thanks for the cleanup. I have reviewed the series and it looks good to me. I would suggest to fold similar changes like, - PATCH 2/5 and PATCH 4/5 into one patch - PATCH 3/5 and PATCH 5/5 into one patch With this update you can add my ack for this series Ok, will do, thanks for the comments. Regards, Omar -- 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] OMAP3: l3: fix for irq 10: nobody cared message
Hi, On Mon, Mar 28, 2011 at 12:38 AM, Santosh Shilimkar santosh.shilim...@ti.com wrote: If an error occurs in the L3 on any other initiator than MPU, the interrupt goes unhandled given that the 'base' register was calculated with the initialized err_base value (which coincidentally points to MPU) and not with the actual source of the error. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com Patch looks good. Did you observe this with DSP initiator?? Yes, when loading a base image for the DSP, I got an: In-band Error Error seen by IVA_SS at address 0; after this fix. I was planning to remove the duplicated Error print too. Acked-by: Santosh Shilimkar santosh.shilim...@ti.com Thanks, I'll add it in my next version when I make the changes affecting only these lines. Regards, Omar -- 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] iommu: Prevent oops in iommu_get() and while arch_iommu is in use
Hi, On Sun, Mar 27, 2011 at 12:27 PM, Sakari Ailus sakari.ai...@maxwell.research.nokia.com wrote: Ramirez Luna, Omar wrote: On Fri, Mar 25, 2011 at 10:13 AM, Sakari Ailus sakari.ai...@maxwell.research.nokia.com wrote: This patchset is aimed to fix a problem in arch_iommu implementation references. When an actual arch_iommu implementation is not loaded while iommu_get() is being called results to a kernel oops, as well as removing an arch_iommu implementation which is in use. How about fixing the dependency instead? Right now iommu2 depends on iommu because of the calls to install_iommu_arch/uninstall_iommu_arch... we should change that dependency to iommu depend on iommu2. Something like iommu (plat) querying iommu2 (mach) for devices to install. There is no direct dependency from a driver using the generic API to a particular implementation of the iommu. This comes from the design of the iommu framework. The generic layer shouldn't depend on particular implementation(s). IMHO there is, take as an example bridgedriver (it is arm/omap dependent), so it depends on iommu providing the mach-omap2 implementation. I imagine isp for omap imposes the same dependency, even more your patchset enforces that dependency. Basically, if there is no arch_iommu the iommu driver does not work, and if there was an arch_iommu but it was removed then the driver crashes. Now, there could be architectures that does not depend on a particular implementation but this iommu driver doesn't support them because if there is no arch_iommu operations, it does nothing or crashes. What comes to the patch, it works as long as there's only one iommu implementation loaded / compiled to the kernel. I wonder if this kind of limitation can be accepted. Which is the way iommu choose to work, like I said if there is no arch_iommu nothing works, most of APIs in iommu depend on an machine specific implementation. To fix that it is not the scope of my proposal. If indeed iommu can function without a machine specific implementation then a redesign needs to be made, but to me the same approach as I did needs to be followed: if there is mach implementation (e.g.: iommu2.c) the generic API needs to depend on it, otherwise the module can be removed and crash the kernel; OTOH if there is no mach implementation, then iommu should not depend on it to be installed as you point out, this could be handled in plat/iommu.h among with: #if defined(CONFIG_ARCH_OMAP1) #error iommu for this processor not implemented yet #else #include plat/iommu2.h #endif A new else defining the install/uninstall_arch_iommu functions or simply reversing the check to be OMAP2+ and error on anything else. Regards, Omar -- 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] OMAP3: l3: fix for irq 10: nobody cared message
On Sun, Mar 27, 2011 at 8:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Mar 26, 2011 at 2:29 AM, Omar Ramirez Luna omar.rami...@ti.com wrote: If an error occurs in the L3 on any other initiator than MPU, the interrupt goes unhandled given that the 'base' register was calculated with the initialized err_base value (which coincidentally points to MPU) and not with the actual source of the error. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-omap2/omap_l3_smx.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_l3_smx.c b/arch/arm/mach-omap2/omap_l3_smx.c index 5f2da75..da917c2 100644 --- a/arch/arm/mach-omap2/omap_l3_smx.c +++ b/arch/arm/mach-omap2/omap_l3_smx.c @@ -196,11 +196,12 @@ static irqreturn_t omap3_l3_app_irq(int irq, void *_l3) /* No timeout error for debug sources */ } - base = ((l3-rt) + (*(omap3_l3_bases[int_type] + err_source))); - /* identify the error source */ for (err_source = 0; !(status (1 err_source)); err_source++) ; + + base = ((l3-rt) + (*(omap3_l3_bases[int_type] + err_source))); + error = omap3_l3_readll(base, L3_ERROR_LOG); One extra space too much. Between base and error assignments? Yep, I can remove it. Regards, Omar -- 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] OMAP3: l3: fix for irq 10: nobody cared message
On Sat, Mar 26, 2011 at 4:38 PM, Sergei Shtylyov sshtyl...@mvista.com wrote: Hello. On 26-03-2011 3:29, Omar Ramirez Luna wrote: If an error occurs in the L3 on any other initiator than MPU, the interrupt goes unhandled given that the 'base' register was calculated with the initialized err_base value (which coincidentally points to MPU) and not with the actual source of the error. Signed-off-by: Omar Ramirez Lunaomar.rami...@ti.com --- arch/arm/mach-omap2/omap_l3_smx.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-omap2/omap_l3_smx.c b/arch/arm/mach-omap2/omap_l3_smx.c index 5f2da75..da917c2 100644 --- a/arch/arm/mach-omap2/omap_l3_smx.c +++ b/arch/arm/mach-omap2/omap_l3_smx.c @@ -196,11 +196,12 @@ static irqreturn_t omap3_l3_app_irq(int irq, void *_l3) /* No timeout error for debug sources */ } - base = ((l3-rt) + (*(omap3_l3_bases[int_type] + err_source))); - /* identify the error source */ for (err_source = 0; !(status (1 err_source)); err_source++) ; + + base = ((l3-rt) + (*(omap3_l3_bases[int_type] + err_source))); + What's the point of having () around rvalue? You could drop them, while at it... You're right, will do. Regards, Omar -- 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: iovmm: fix SW flags passed by user
On Fri, Mar 25, 2011 at 12:30 PM, Sergei Shtylyov sshtyl...@mvista.com wrote: Hello. Omar Ramirez Luna wrote: Commit d038aee24dcd changes iovmm to receive flags specified by user, Please also specify that commit's summary for the human readers. Bare ID is only usable to gitweb... Here it is the patch, introducing the change: https://patchwork.kernel.org/patch/620871/ If accepted by Hiroshi, I can update the description making reference to this patch instead of commit. Regards, Omar -- 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] iommu: Prevent oops in iommu_get() and while arch_iommu is in use
On Fri, Mar 25, 2011 at 10:13 AM, Sakari Ailus
sakari.ai...@maxwell.research.nokia.com wrote:
Hi,
This patchset is aimed to fix a problem in arch_iommu implementation
references. When an actual arch_iommu implementation is not loaded while
iommu_get() is being called results to a kernel oops, as well as
removing an arch_iommu implementation which is in use.
How about fixing the dependency instead? Right now iommu2 depends on
iommu because of the calls to
install_iommu_arch/uninstall_iommu_arch... we should change that
dependency to iommu depend on iommu2. Something like iommu (plat)
querying iommu2 (mach) for devices to install.
This way depmod (if I'm not mistaken) can do its job, you won't be
able to remove iommu2 in the middle of execution nor install iommu
without its mach counterpart being there first, it should also fix
clients depending on this modules, e.g modprobe bridgedriver would
only install iommu and bridgedriver, with this new dependency iommu2
should be installed as well. BTW same happens with omap mailbox.
$ lsmod
iovmm 7225 1 bridgedriver
iommu 11084 2 bridgedriver,iovmm
iommu2 4783 1 iommu
I can send as a patch if the mailer screws the spacing, also just
copy-pasted and played with the pointers, if needed we can give better
naming.
Regards,
Omar
---
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index adb083e..ab2f9a9 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -341,15 +341,47 @@ static const struct iommu_functions omap2_iommu_ops = {
.dump_ctx = omap2_iommu_dump_ctx,
};
+/**
+ * install_iommu_arch - Install archtecure specific iommu functions
+ * @ops: a pointer to architecture specific iommu functions
+ *
+ * There are several kind of iommu algorithm(tlb, pagetable) among
+ * omap series. This interface installs such an iommu algorighm.
+ **/
+int install_iommu_arch(const struct iommu_functions **ops)
+{
+ if (*ops)
+ return -EBUSY;
+ *ops = omap2_iommu_ops;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(install_iommu_arch);
+
+/**
+ * uninstall_iommu_arch - Uninstall archtecure specific iommu functions
+ * @ops: a pointer to architecture specific iommu functions
+ *
+ * This interface uninstalls the iommu algorighm installed previously.
+ **/
+void uninstall_iommu_arch(const struct iommu_functions **ops)
+{
+ if (*ops != omap2_iommu_ops)
+ pr_err(%s: not your arch\n, __func__);
+
+ *ops = NULL;
+}
+EXPORT_SYMBOL_GPL(uninstall_iommu_arch);
+
static int __init omap2_iommu_init(void)
{
- return install_iommu_arch(omap2_iommu_ops);
+ return 0;
}
module_init(omap2_iommu_init);
static void __exit omap2_iommu_exit(void)
{
- uninstall_iommu_arch(omap2_iommu_ops);
+ /* Do nothing */
}
module_exit(omap2_iommu_exit);
diff --git a/arch/arm/plat-omap/include/plat/iommu.h
b/arch/arm/plat-omap/include/plat/iommu.h
index 174f1b9..1c8e7ee 100644
--- a/arch/arm/plat-omap/include/plat/iommu.h
+++ b/arch/arm/plat-omap/include/plat/iommu.h
@@ -177,9 +177,6 @@ extern int iommu_set_isr(const char *name,
extern void iommu_save_ctx(struct iommu *obj);
extern void iommu_restore_ctx(struct iommu *obj);
-extern int install_iommu_arch(const struct iommu_functions *ops);
-extern void uninstall_iommu_arch(const struct iommu_functions *ops);
-
extern int foreach_iommu_device(void *data,
int (*fn)(struct device *, void *));
diff --git a/arch/arm/plat-omap/include/plat/iommu2.h
b/arch/arm/plat-omap/include/plat/iommu2.h
index 10ad05f..8189f58 100644
--- a/arch/arm/plat-omap/include/plat/iommu2.h
+++ b/arch/arm/plat-omap/include/plat/iommu2.h
@@ -80,6 +80,9 @@
#define MMU_RAM_MIXED_MASK (1 MMU_RAM_MIXED_SHIFT)
#define MMU_RAM_MIXED MMU_RAM_MIXED_MASK
+extern int install_iommu_arch(const struct iommu_functions **ops);
+extern void uninstall_iommu_arch(const struct iommu_functions **ops);
+
/*
* register accessors
*/
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index 8a51fd5..f088929 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -37,38 +37,6 @@ static struct platform_driver omap_iommu_driver;
static struct kmem_cache *iopte_cachep;
/**
- * install_iommu_arch - Install archtecure specific iommu functions
- * @ops: a pointer to architecture specific iommu functions
- *
- * There are several kind of iommu algorithm(tlb, pagetable) among
- * omap series. This interface installs such an iommu algorighm.
- **/
-int install_iommu_arch(const struct iommu_functions *ops)
-{
- if (arch_iommu)
- return -EBUSY;
-
- arch_iommu = ops;
- return 0;
-}
-EXPORT_SYMBOL_GPL(install_iommu_arch);
-
-/**
- * uninstall_iommu_arch - Uninstall archtecure specific iommu functions
- * @ops: a pointer to architecture specific iommu functions
- *
- * This
Re: [PATCH] OMAP: iovmm: fix SW flags passed by user
On Fri, Mar 25, 2011 at 3:05 PM, Russell King - ARM Linux li...@arm.linux.org.uk Commit d038aee24dcd changes iovmm to receive flags specified by user, Please also specify that commit's summary for the human readers. Bare ID is only usable to gitweb... Here it is the patch, introducing the change: https://patchwork.kernel.org/patch/620871/ If accepted by Hiroshi, I can update the description making reference to this patch instead of commit. It is a requirement that all descriptions which refer to a commit shall contain both the commit ID *and* the summary line for that commit. Failure to ensure that's the case results in rejection of the patch until it conforms. Not a problem then... resending v2. Regards, Omar -- 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] OMAP3: l3: fix for irq 10: nobody cared message
On Fri, Mar 25, 2011 at 7:29 PM, Omar Ramirez Luna omar.rami...@ti.com wrote: If an error occurs in the L3 on any other initiator than MPU, the interrupt goes unhandled given that the 'base' register was calculated with the initialized err_base value (which s/err_base/err_source/ coincidentally points to MPU) and not with the actual source of the error. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com Regards, Omar -- 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: [RESEND PATCH] staging: tidspbridge: protect dmm_map properly
Hi Felipe, On Fri, Mar 11, 2011 at 11:38 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Mar 11, 2011 at 7:30 PM, Felipe Contreras felipe.contre...@nokia.com wrote: We need to protect not only the dmm_map list, but the individual map_obj's, otherwise, we might be building the scatter-gather list with garbage. So, use the existing proc_lock for that. I observed race conditions which caused kernel panics while running stress tests, also, Tuomas Kulve found it happening quite often in Gumstix Over. This patch fixes those. Cc: Tuomas Kulve tuo...@kulve.fi Signed-off-by: Felipe Contreras felipe.contre...@nokia.com Omar, 2.6.38 is imminent, if this patch has any chance of getting in, I think you would need to push it soon. I'll push it to my queue, however I don't think this would be even considered to make it before the official 2.6.38 which should happen in the next few days, given that it should climb to staging tree first and from there to mainline... we could attempt to shot directly to the mainline kernel, but that would raise a few eyebrows and in the end be rejected as well because this is a staging driver which is still in process of cleanup. OTOH, this patch was there long ago, since December, but I just lost the track of it at the moment, along with the other proposed solutions, so I take it as my fault for not picking it up then. Regards, Omar -- 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: [RESEND PATCH] staging: tidspbridge: protect dmm_map properly
Hi, On Fri, Mar 11, 2011 at 12:31 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Well, not crashing is better than crashing, regardless of the status of the driver. And there's no reason not to include the fix. No reason indeed. But if you send it to Greg, then it's Greg's call. Ok then. Regards, Omar -- 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 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp
Hi Benoit, On Tue, Mar 8, 2011 at 3:29 PM, Cousson, Benoit b-cous...@ti.com wrote: What I meant is that the mmu does not have any explicit PRCM module enable control. The PRCM does control only the DSP as a whole. Since hwmods are generated with a PRCM granularity, any sub module will not be exposed by default. Ok, got it. Thanks for the reply. I've checked both IPU and DSP HW specs, and in the both cases, the mmu IP is the only module with OCP port from the MPU in the relevant subsystem. In that case, it is easier for me to remove that DSP and IPU hwmods and move everything under the MMUs hwmods. It will then highlight the re-use of the same IP in two different subsystems. I have now to modify the OMAP4 generator in order to provide the correct data in the right format. I'll try to do that before the end of this week. This will not have any real impact for you except for the name change (mmu_ipu and mmu_dsp). Is that OK for you? No problem, once it is ready I'll update OMAP3 to mimic the name style. Thanks, Omar -- 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 2/4] OMAP4: hwmod data: add mmu hwmod for ipu and dsp
Hi Benoit, On Mon, Mar 7, 2011 at 6:55 AM, Cousson, Benoit b-cous...@ti.com wrote: Hi Omar, I have some concern about the introduction of a hwmod that does not match the actual HW capability. MMU does exist, but there is no SW control for it. Maybe I'm missing something, but iommu (driver) is meant to control isp, iva, ipu and dsp MMUs; even with a simple driver interfaced with iommu, that had nothing to do with the modules mentioned, you could still deassert the reset, enable the clocks, create your tables and add entries, and so on... not that it would be useful for anybody other than the real HW containing the MMU subsystem. In fact the only control available is for mmu + cache + logic, and that's why the MMU is handle today under the main DSP/IPU hwmod. AFAIK, sysc configuration is missing from the old hwmods, I thought separate hwmods gave: - flexibility: so the system wouldn't dump_stack trying to read mmu registers, because the user doesn't know ipu/dsp code should handle the reset first. - clarity: so iommu handles its own mmu hwmods instead of hard coding the names of the pseudo hwmods containing the mmu. Here you are just duplicating dsp_hwmod and ipu_hwmod with dsp_mmu_hwmod / ipu_mmu_hwmod and adding some memory space for the mmu part. In that case, you can still use the previous name and add the missing entries in it. The only advantage I can see is the usage of a common class that will allow you to handle both DSP and IPU using the same MMU driver. So, what are you going to do with the remaining entries for dsp_hwmod and ipu_hwmod? I think these can be removed, and iommu code can handle its own hwmods; but if you want to update the old ones, that can be done too, the tradeoff would be that iommu needs to know the name of the hwmods with mmu data. Regards, Omar -- 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: protect dmm_map properly
Hi Felipe, On Mon, Mar 7, 2011 at 12:02 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Dec 20, 2010 at 7:12 PM, Felipe Contreras felipe.contre...@nokia.com wrote: We need to protect not only the dmm_map list, but the individual map_obj's, otherwise, we might be building the scatter-gather list with garbage. So, use the existing proc_lock for that. I observed race conditions which caused kernel panics while running stress tests. This patch fixes those. I just heard that Tuomas Kulve is getting a lot of panics on Gumstix Overo. I propose we apply this patch on the stable tree ASAP, and if there's no better proposals, also on .38. Can you or Tuomas share the bug report data (panic log, test case maybe)? I would like to discard issues affected by timing that could be hidden with this patch. I agree that for the time being this needs to be sent upstream, even if in paper Ohad's patch solves the issue without side effects of locking. Thanks, Omar -- 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+: hwmod: use status bit info for reset line
Hi Paul On Fri, Mar 4, 2011 at 3:36 PM, Paul Walmsley p...@pwsan.com wrote: It's after our -rc6 cutoff, but I'll take it anyway for 2.6.39. Made a few minor changes -- updated patch below -- if you have a chance, please give it a quick test. The 'integration-2.6.39' branch at git://git.pwsan.com/linux-integration now contains it. Or if you have any comments about the changes, those are welcome too. Agree with changes, didn't see any issue either while testing. Regards, Omar -- 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 v3 2/2] OMAP: IOMMU: add support to callback during fault handling
Hi,
On Wed, Feb 16, 2011 at 1:35 PM, David Cohen daco...@gmail.com wrote:
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 49a1e5e..adb083e 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
...
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
+ if (stat MMU_IRQ_TLBMISS)
+ errs |= OMAP_IOMMU_ERR_TLB_MISS;
+ if (stat MMU_IRQ_TRANSLATIONFAULT)
+ errs |= OMAP_IOMMU_ERR_TRANS_FAULT;
+ if (stat MMU_IRQ_EMUMISS)
+ errs |= OMAP_IOMMU_ERR_EMU_MISS;
+ if (stat MMU_IRQ_TABLEWALKFAULT)
+ errs |= OMAP_IOMMU_ERR_TBLWALK_FAULT;
+ if (stat MMU_IRQ_MULTIHITFAULT)
+ errs |= OMAP_IOMMU_ERR_MULTIHIT_FAULT;
I still don't think this adds any value, generic layer and omap
errors are the same thing in this case... OTOH OMAP 1710 (not
supported by iommu yet) has the following bits:
3 Prefetch_err
2 Perm_fault
1 Tlb_miss
0 Trans_fault
They don't match any of your generic layer errors masks for reading,
hence more generic errors will need to be defined, and then more OMAP#
masks... I think we just need to stick with the mach specific errors,
and let mach code handle its specifics when reporting.
But anyway it is just me...
diff --git a/arch/arm/plat-omap/iommu.c b/arch/arm/plat-omap/iommu.c
index f55f458..b0e0efc 100644
--- a/arch/arm/plat-omap/iommu.c
+++ b/arch/arm/plat-omap/iommu.c
@@ -780,25 +780,19 @@ static void iopgtable_clear_entry_all(struct iommu *obj)
*/
static irqreturn_t iommu_fault_handler(int irq, void *data)
{
- u32 stat, da;
+ u32 da, errs;
u32 *iopgd, *iopte;
- int err = -EIO;
struct iommu *obj = data;
if (!obj-refcount)
return IRQ_NONE;
- /* Dynamic loading TLB or PTE */
- if (obj-isr)
- err = obj-isr(obj);
-
- if (!err)
- return IRQ_HANDLED;
-
clk_enable(obj-clk);
- stat = iommu_report_fault(obj, da);
+ errs = iommu_report_fault(obj, da);
clk_disable(obj-clk);
- if (!stat)
+
+ /* Fault callback or TLB/PTE Dynamic loading */
+ if (obj-isr !obj-isr(obj, da, errs, obj-isr_priv))
return IRQ_HANDLED;
BTW, why not changing the name isr for cb, it is confusing since there
is another fault_isr called by mmu, AFAIK nobody uses obj-isr
iommu_disable(obj);
@@ -806,15 +800,16 @@ static irqreturn_t iommu_fault_handler(int irq, void
*data)
iopgd = iopgd_offset(obj, da);
if (!iopgd_is_table(*iopgd)) {
- dev_err(obj-dev, %s: da:%08x pgd:%p *pgd:%08x\n, obj-name,
- da, iopgd, *iopgd);
+ dev_err(obj-dev, %s: errs:0x%08x da:0x%08x pgd:0x%p
+ *pgd:px%08x\n, obj-name, errs, da, iopgd, *iopgd);
return IRQ_NONE;
}
iopte = iopte_offset(iopgd, da);
- dev_err(obj-dev, %s: da:%08x pgd:%p *pgd:%08x pte:%p *pte:%08x\n,
- obj-name, da, iopgd, *iopgd, iopte, *iopte);
+ dev_err(obj-dev, %s: errs:0x%08x da:0x%08x pgd:0x%p *pgd:0x%08x
+ pte:0x%p *pte:0x%08x\n, obj-name, errs, da, iopgd, *iopgd,
+ iopte, *iopte);
return IRQ_NONE;
}
@@ -917,6 +912,33 @@ void iommu_put(struct iommu *obj)
}
EXPORT_SYMBOL_GPL(iommu_put);
+int iommu_set_isr(const char *name,
+ int (*isr)(struct iommu *obj, u32 da, u32 iommu_errs,
+ void *priv),
+ void *isr_priv)
So I think the following will be bad practice:
mmu = iommu_get(iva2);
if (!IS_ERR(mmu))
mmu-isr = mmu_fault_callback;
Shall we think anything to prevent such mis-usage?
Regards,
Omar
--
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 v3 2/2] OMAP: IOMMU: add support to callback during fault handling
Hi, On Mon, Feb 21, 2011 at 3:12 PM, David Cohen daco...@gmail.com wrote: Generic errors codes make easier to threat possible IOMMU users which have (partially or totally) common drivers for different OMAP versions. Agree then. + /* Fault callback or TLB/PTE Dynamic loading */ + if (obj-isr !obj-isr(obj, da, errs, obj-isr_priv)) return IRQ_HANDLED; BTW, why not changing the name isr for cb, it is confusing since there is another fault_isr called by mmu, AFAIK nobody uses obj-isr The main purpose of this function is to be an ISR, not only callback. Yep, I just thought it was a bit too much of: (1) The real isr: iommu_fault_handler, set on request_irq (2) A plugged fault_isr for mach specific code: omap2_iommu_fault_isr, for error reporting. (3) And then a plugged custom isr, to be set by the user. Feel free to ignore. So I think the following will be bad practice: mmu = iommu_get(iva2); if (!IS_ERR(mmu)) mmu-isr = mmu_fault_callback; Shall we think anything to prevent such mis-usage? Well, the IOMMU user has access to IOMMU obj, so it can not only change the (*isr)() but to mess with a lot of other stuff. The only way to prevent it is to avoid user to have obj. But then, this fix (or issue) does not belong to this patch. Agree, just pointing out. Regards, Omar -- 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 v5 3/5] OMAP4: hwmod data: add mailbox data
Hi Tony, On Thu, Feb 17, 2011 at 5:39 PM, Tony Lindgren t...@atomide.com wrote: * Ramirez Luna, Omar omar.rami...@ti.com [110215 16:11]: If you have to do several platform_get_irq_byname to get this one, I'd prefer to get rid of that name for OMAP4. It will make mailbox irq consistent with the other hwmods. I was thinking to standardize the names to be mbox0..mboxN across all the platforms, reason being that the mailbox also has capabilities to be used not only by dsp or iva, by using a polling method. So even if the mailbox in OMAP3 is called dsp, it has 4 more queues apart from the 2 used for messaging between arm and dsp, that could be used even if tidspbridge wasn't there. So what's the status of this series? Can the rest of the patches now be applied on top of the current omap-for-linus or do the names still need fixing? I'm going to remove the names for the hwmods with a single irq. And change the interface clock to be the main_clk, so it can be disabled when runtime pm is disabled. I won't resend the patch for OMAP4 hwmod data, as this was send earlier by Benoit. Regards, Omar -- 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 v5 0/5] omap: mailbox: hwmod support
Hi Kevin, Benoit, On Fri, Feb 11, 2011 at 5:01 PM, Kevin Hilman khil...@ti.com wrote: Omar Ramirez Luna omar.rami...@ti.com writes: Mailbox hwmod support for OMAP 2,3,4. This was tested on OMAP3 (3430, 3630), minor testing was made on OMAP4. No testing on OMAP2 since I don't have the hardware. To help in testing, I wrote a simple mailbox loopback test module for OMAP2/3/4 that I used to do send and receive messages on the MPU. This can be used to test the mailbox without any DSP software. I tested it against l-o master branch and found a couple bugs in the mailbox driver (patches posted earlier today.) With those patches plus my test I can send receieve a series of messages on the MPU, which is enough to sanity test the basic sending and receiving messages on the MPU. I've tested the master branch, now it's your turn to use this test module to validate this hmod conversion series. The test module is available here: git://gitorious.org/omap-test/mailbox.git I rechecked on OMAP3 (zoom2, zoom3) and OMAP4 (blaze), and functionality hasn't changed with the hwmod support + Kevin's mbox test. On OMAP3, although functionality is ok with this patch set, I noticed that the interface clock is not being disabled with its corresponding pm_runtime_disable call. Previously, the driver enabled/disabled the ick clock on demand, but now the clock stays enabled always (only affected by the smart-idle feature, I presume). Should this clock be placed in the main_clk field of omap_hwmod struct instead of omap_hwmod_ocp_if, I moved it to ocp_if because of the ick thing, any comment? Regards, Omar -- 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 v5 3/5] OMAP4: hwmod data: add mailbox data
Hi Benoit,
On Mon, Feb 14, 2011 at 9:00 AM, Cousson, Benoit b-cous...@ti.com wrote:
+static struct omap_hwmod_irq_info omap44xx_mailbox_irqs[] = {
+ { .name = mbox, .irq = 26 + OMAP44XX_IRQ_GIC_START, },
The original entry was unnamed since it is an unique entry and thus does not
need to be differentiate on this platform.
{ .irq = 26 + OMAP44XX_IRQ_GIC_START },
Do you really need to have a name here? The strategy being to provide a name
only if more than one entry exist.
It is perfectibility doable, I'm just trying to understand your rational.
It is this way instead of plain platform_get_irq because omap2420 has
two interrupt sources to MPU and mailbox driver uses
platform_get_irq_byname to get the irq number.
Regards,
Omar
--
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 v5 3/5] OMAP4: hwmod data: add mailbox data
Hi,
On Tue, Feb 15, 2011 at 4:05 PM, Cousson, Benoit b-cous...@ti.com wrote:
It is this way instead of plain platform_get_irq because omap2420 has
two interrupt sources to MPU and mailbox driver uses
platform_get_irq_byname to get the irq number.
This is what I was thinking, except that on OMAP2420 the names are:
+ { .name = dsp, .irq = 26, },
+ { .name = iva, .irq = 34, },
and on OMAP2430 and OMAP3
+ { .name = dsp, .irq = 26, },
so why is it named mbox on OMAP4?
I'm not very familiar with OMAP4 terminology... but IMHO, I guess
naming it dsp, would imply that this is a mailbox for the dsp, when
the interrupt can be generated by either the M3, dsp, I think even the
IVA can write into it.
If you have to do several platform_get_irq_byname to get this one, I'd
prefer to get rid of that name for OMAP4. It will make mailbox irq
consistent with the other hwmods.
I was thinking to standardize the names to be mbox0..mboxN across all
the platforms, reason being that the mailbox also has capabilities to
be used not only by dsp or iva, by using a polling method.
So even if the mailbox in OMAP3 is called dsp, it has 4 more queues
apart from the 2 used for messaging between arm and dsp, that could be
used even if tidspbridge wasn't there.
Did I get you correctly?
Regards,
Omar
--
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] OMAP: Cleanup IOMMU error messages
Hi David,
On Mon, Jan 31, 2011 at 11:25 AM, David Cohen david.co...@nokia.com wrote:
IOMMU error messages are duplicated. They're printed on IOMMU specific
layer for OMAP2,3 and once again on the above layer. With this patch,
the error message is printed on the above layer only.
So, you say they are duplicated, previously the type of fault was
printed in the omap2,3 code and the addresses involving the error
printed on plat code... with this patch both messages are printed on
plat code, I don't see where the duplication/fix is about.
AFAIK, your patch removes this line:
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
Which I assume is the one being printed again in plat code, right?
Signed-off-by: David Cohen david.co...@nokia.com
---
arch/arm/mach-omap2/iommu2.c | 33 +--
arch/arm/plat-omap/include/plat/iommu.h | 2 +-
arch/arm/plat-omap/iommu.c | 36 --
3 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..bb3d75b 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -143,33 +143,32 @@ static void omap2_iommu_set_twl(struct iommu *obj, bool
on)
__iommu_set_twl(obj, false);
}
-static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
+static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra, u32 *iommu_errs)
{
- int i;
+ u32 errs = 0;
u32 stat, da;
- const char *err_msg[] = {
- tlb miss,
- translation fault,
- emulation miss,
- table walk fault,
- multi hit fault,
- };
AFAIU, this code living in mach-omap2, means that this errors are
common for omap2,3,4 which they are. Does moving them to plat code
implies that all omap platforms expect these errors?
stat = iommu_read_reg(obj, MMU_IRQSTATUS);
stat = MMU_IRQ_MASK;
- if (!stat)
+ if (!stat) {
+ *iommu_errs = 0;
return 0;
+ }
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
-
- for (i = 0; i ARRAY_SIZE(err_msg); i++) {
- if (stat (1 i))
- printk(%s , err_msg[i]);
- }
- printk(\n);
+ if (stat MMU_IRQ_TLBMISS)
+ errs |= IOMMU_ERR_TLB_MISS;
+ if (stat MMU_IRQ_TRANSLATIONFAULT)
+ errs |= IOMMU_ERR_TRANS_FAULT;
+ if (stat MMU_IRQ_EMUMISS)
+ errs |= IOMMU_ERR_EMU_MISS;
+ if (stat MMU_IRQ_TABLEWALKFAULT)
+ errs |= IOMMU_ERR_TBLWALK_FAULT;
+ if (stat MMU_IRQ_MULTIHITFAULT)
+ errs |= IOMMU_ERR_MULTIHIT_FAULT;
+ *iommu_errs = errs;
Is there any point in doing this?
Basically: *iommu_errs = stat, given that the mask values and the
error defines are the same for each case.
Besides I haven't seen two errors trigger a single interrupt.
...
- iopte = iopte_offset(iopgd, da);
-
- dev_err(obj-dev, %s: da:%08x pgd:%p *pgd:%08x pte:%p *pte:%08x\n,
- __func__, da, iopgd, *iopgd, iopte, *iopte);
+ for (i = 0; i ARRAY_SIZE(err_msg); i++) {
+ if (errs (1 i))
+ printk(KERN_CONT %s, err_msg[i]);
+ }
+ printk(\n);
I think we should get rid of the printks.
Regards,
Omar
--
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 00/12]staging:tidspbridge - Remove hungarian notation from structures
On Mon, Jan 17, 2011 at 9:19 PM, Armando Uribe x0095...@ti.com wrote: This series of patches remove the hungarian notation from the structure's elements. Also the whole patch series has been rebased to the latest of tidspbridge tree, and minor corrections were made from the original version of Rene Sapiens. Pushed to dspbridge. Regards, Omar -- 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: [PATCHv2] staging: tidspbridge: configure full L1 MMU range
Hi, On Wed, Jan 5, 2011 at 3:25 PM, Fernando Guzman Lugo fernando.l...@ti.com wrote: From: Guzman Lugo, Fernando fernando.l...@ti.com IVA MMU can manage up to 4GB of address space through its page tables, given that it's L1 is divided into 1MB sections it requires at least 16KB for its table which represents 4096 entries of 32 bits each. Previously, only 1GB was being handled by setting the page table size to 4KB, any virtual address beyond of the L1 size used, would fall into memory that does not belong to L1 translation tables, leading to unpredictable results. So, set the L1 table size to cover the entire MMU range (4GB) whether is meant to be used or not. Reported-by: Felipe Contreras felipe.contre...@nokia.com Signed-off-by: Fernando Guzman Lugo fernando.l...@ti.com Signed-off-by: Felipe Contreras felipe.contre...@nokia.com Nice fix, pushed to for-gkh-2.6.38 branch. Regards, Omar -- 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 - configure full L1 MMU range
Hi,
On Wed, Jan 5, 2011 at 12:20 AM, Fernando Guzman Lugo
fernando.l...@ti.com wrote:
Otherwise a virtual address beyond of the L1 size is used,
the MMU hardware will look into a memory that does not belong to
L1 translation tables. IOW; the MMU would allow to access any
memory, configured or not.
How about:
IVA MMU can manage up to 4GB of address space through its page tables,
given that it's L1 is divided into 1MB sections it requires at least
16KB for its table which represents 4096 entries of 32 bits each.
Previously, only 1GB was being handled by setting the page table size
to 4KB, any virtual address beyond of the L1 size used, would fall
into memory that does not belong to L1 translation tables, leading to
unpredictable results.
So, set the L1 table size to cover the entire MMU range (4GB) whether
is meant to be used or not.
Reported-by: Felipe Contreras felipe.contre...@nokia.com
Signed-off-by: Fernando Guzman Lugo fernando.l...@ti.com
Signed-off-by: Felipe Contreras felipe.contre...@nokia.com
---
drivers/staging/tidspbridge/core/tiomap3430.c | 6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c
b/drivers/staging/tidspbridge/core/tiomap3430.c
index 1be081f..ec96d1e 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -70,6 +70,7 @@
#define MMU_LARGE_PAGE_MASK 0x
#define MMU_SMALL_PAGE_MASK 0xF000
#define OMAP3_IVA2_BOOTADDR_MASK 0xFC00
+#define MMU_L1_SIZE 0x4000
#define PAGES_II_LVL_TABLE 512
#define PHYS_TO_PAGE(phys) pfn_to_page((phys) PAGE_SHIFT)
@@ -786,10 +787,7 @@ static int bridge_dev_create(struct bridge_dev_context
pt_attrs = kzalloc(sizeof(struct pg_table_attrs), GFP_KERNEL);
if (pt_attrs != NULL) {
- /* Assuming that we use only DSP's memory map
- * until 0x4000: , we would need only 1024
- * L1 enties i.e L1 size = 4K */
- pt_attrs-l1_size = 0x1000;
+ pt_attrs-l1_size = MMU_L1_SIZE;
How about using SZ_16K instead, struct member name already specifies
it is L1 size, is the new define needed?
Regards,
Omar
--
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 - Remove unused defined constants
Hi, On Thu, Dec 16, 2010 at 7:18 PM, Armando Uribe x0095...@ti.com wrote: Remove defined constants not being used. Signed-off-by: Armando Uribe x0095...@ti.com Pushed the entire series to dspbridge. Regards, Omar -- 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: remove code referred by OPT_ZERO_COPY_LOADER
Hi, On Mon, Dec 20, 2010 at 3:23 PM, Ernesto Ramos erne...@ti.com wrote: Remove code referred by OPT_ZERO_COPY_LOADER since it is not used. Signed-off-by: Ernesto Ramos erne...@ti.com --- drivers/staging/tidspbridge/dynload/cload.c | 60 +-- 1 files changed, 20 insertions(+), 40 deletions(-) Pushed to dspbridge. Regards, Omar -- 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] staging: tidspbridge: rmgr/node.c code cleanup
On Thu, Dec 9, 2010 at 3:47 PM, Ionut Nicu ionut.n...@gmail.com wrote: Reorganized some code in rmgr/node.c to increase its readability. Most of the changes reduce the code indentation level and simplifiy the code. No functional changes were done. Signed-off-by: Ionut Nicu ionut.n...@mindbit.ro --- drivers/staging/tidspbridge/rmgr/node.c | 607 ++ 1 files changed, 283 insertions(+), 324 deletions(-) Pushed to dspbridge. Regards, Omar -- 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: rmgr code cleanup
Hi Ionut,
On Thu, Dec 9, 2010 at 3:47 PM, Ionut Nicu ionut.n...@gmail.com wrote:
Reorganized some code in the rmgr module to increase
its readability. No functional changes were done.
Signed-off-by: Ionut Nicu ionut.n...@mindbit.ro
---
drivers/staging/tidspbridge/rmgr/drv.c | 171 +---
drivers/staging/tidspbridge/rmgr/node.c | 81 +-
drivers/staging/tidspbridge/rmgr/rmm.c | 266 +-
3 files changed, 229 insertions(+), 289 deletions(-)
diff --git a/drivers/staging/tidspbridge/rmgr/drv.c
b/drivers/staging/tidspbridge/rmgr/drv.c
index e0fc895..71a9489 100644
--- a/drivers/staging/tidspbridge/rmgr/drv.c
+++ b/drivers/staging/tidspbridge/rmgr/drv.c
@@ -304,40 +304,28 @@ int drv_proc_update_strm_res(u32 num_bufs, void
*strm_resources)
...
@@ -608,40 +596,28 @@ int drv_request_resources(u32 dw_context, u32
*dev_node_strg)
DBC_REQUIRE(dw_context != 0);
DBC_REQUIRE(dev_node_strg != NULL);
- /*
- * Allocate memory to hold the string. This will live untill
- * it is freed in the Release resources. Update the driver object
- * list.
- */
-
if (!drv_datap || !drv_datap-drv_object)
- status = -ENODATA;
- else
- pdrv_object = drv_datap-drv_object;
-
- if (!status) {
- pszdev_node = kzalloc(sizeof(struct drv_ext), GFP_KERNEL);
- if (pszdev_node) {
- strncpy(pszdev_node-sz_string,
- (char *)dw_context, MAXREGPATHLENGTH - 1);
- pszdev_node-sz_string[MAXREGPATHLENGTH - 1] = '\0';
- /* Update the Driver Object List */
- *dev_node_strg = (u32) pszdev_node-sz_string;
- list_add_tail(pszdev_node-link,
- pdrv_object-dev_node_string);
- } else {
- status = -ENOMEM;
- *dev_node_strg = 0;
- }
- } else {
- dev_dbg(bridge, %s: Failed to get Driver Object from
Registry,
- __func__);
- *dev_node_strg = 0;
- }
+ return -ENODATA;
+
+ pdrv_object = drv_datap-drv_object;
+ *dev_node_strg = 0;
+
+ pszdev_node = kzalloc(sizeof(struct drv_ext), GFP_KERNEL);
+ if (!pszdev_node)
+ return -ENOMEM;
+
+ strncpy(pszdev_node-sz_string, (char *)dw_context,
+ MAXREGPATHLENGTH - 1);
+ pszdev_node-sz_string[MAXREGPATHLENGTH - 1] = '\0';
+ *dev_node_strg = (u32) pszdev_node-sz_string;
+
+ /* Update the Driver Object List */
+ list_add_tail((struct list_head *)pszdev_node,
+ pdrv_object-dev_node_string);
I'm not quite comfortable casting to list_head ptr, given that there
is already a member of that type, if the struct type of pszdev_node is
moved around and the first member changes, there should be issues.
Any reason to use them in this patch?
Regards,
Omar
--
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] dspbridge: Fix atoi to support hexadecimal numbers correctly
On Sun, Dec 12, 2010 at 7:39 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: For some strange reason, the DSP base image node/object properties description string stores hexadecimal numbers with a 'h' or 'H' suffix instead of a '0x' prefix. This causes parsing issue because the dspbridge atoi() implementation relies on strict_strtoul(), which will return an error because of the trailing 'h' character. As the atoi() return value is never checked for an error anyway, replace strict_strtoul() with simple_strtoul() to ignore the suffix. This fix gets rid of the following assertion failed messages that were printed when running the dsp-dummy test application. drivers/staging/tidspbridge/rmgr/nldr.c, line 1691: Assertion (segid == MEMINTERNALID || segid == MEMEXTERNALID) failed. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/staging/tidspbridge/rmgr/dbdcd.c | 6 +- 1 files changed, 1 insertions(+), 5 deletions(-) This patch raises a checkpatch warning: WARNING: consider using strict_strtoul in preference to simple_strtoul However on this case, simple_strtoul is preferred as explained in the commit. Pushed to dspbridge. Thanks, Omar -- 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] dspbridge: Fix atoi to support hexadecimal numbers correctly
On Wed, Dec 22, 2010 at 7:00 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Sun, Dec 12, 2010 at 7:39 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: For some strange reason, the DSP base image node/object properties description string stores hexadecimal numbers with a 'h' or 'H' suffix instead of a '0x' prefix. This causes parsing issue because the dspbridge atoi() implementation relies on strict_strtoul(), which will return an error because of the trailing 'h' character. As the atoi() return value is never checked for an error anyway, replace strict_strtoul() with simple_strtoul() to ignore the suffix. This fix gets rid of the following assertion failed messages that were printed when running the dsp-dummy test application. drivers/staging/tidspbridge/rmgr/nldr.c, line 1691: Assertion (segid == MEMINTERNALID || segid == MEMEXTERNALID) failed. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/staging/tidspbridge/rmgr/dbdcd.c | 6 +- 1 files changed, 1 insertions(+), 5 deletions(-) This patch raises a checkpatch warning: WARNING: consider using strict_strtoul in preference to simple_strtoul However on this case, simple_strtoul is preferred as explained in the commit. Pushed to dspbridge. BTW, fixing subject to staging: tidspbridge Regards, Omar -- 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 5/5] OMAP: mailbox: use runtime pm for clk and sysc handling
On Fri, Dec 17, 2010 at 10:28 AM, Cousson, Benoit b-cous...@ti.com wrote: /* SYSCONFIG: register bit definition */ -#define AUTOIDLE (1 0) #define SOFTRESET (1 1) -#define SMARTIDLE (2 3) #define OMAP4_SOFTRESET (1 0) -#define OMAP4_NOIDLE (1 2) -#define OMAP4_SMARTIDLE (2 2) /* SYSSTATUS: register bit definition */ #define RESETDONE (1 0) Is this still required? Yes, mailbox uses the softreset every time it is requested (and that it has no current users at the time), it does it before configuring sysc, as opossed to what this patch does (where clk is enabled, sysc configured, and then softreset), but no harm was seen with this sequence. AFAIK, enabling/disabling hwmod handles prm reset but not softreset bit, that's why I left it there. In fact both are handled now. hardreset is managed for IVA, DSP and IPU, and softreset for most the other IPs. The only slight difference, is that softreset for the moment is just done once at setup time. The idea being that we do not know the state the bootloader configured IP, so we reset it by default. Yes, I noticed that softreset was handled on setup only. I thought I did it for every enable from disable state, but in fact this is not the case, because AFAIK there is not use-case for that yet. Why do you have to softreset the mailbox? That sounds like some old HW bugs from the past that we solve using the softreset. It is adviced in the TRM that before initialization of the module, you should do a softreset and set sysc register, I found this explanation too (with git blame): omap: mailbox: Execute softreset at startup The softreset at startup is introduced as TRM describes and also some register bit definitions are added instead of magic number But mailbox doesn't execute this on init, but on every first user request, it doesn't sound like there was a bug solved with this. Also related (these were the quickest I could check): - The DSI protocol engine can be reset by software. This reset can be done for debug purposes or after a protocol error. - UART needs it too, but in this case it was executed one time at init, so hwmod was the perfect match to remove it. Could you elaborate on that use case? If there is a need, I'd rather update the hwmod core than letting you playing directly with the sysconfig. At least we can expose the API we did for that purpose but didn't merge due to the lack of any strong need. I also thought of this, but then driver would need: pm_runtime_get_sync()// to enable the clock pdata-device_reset() // to set the softreset pm_runtime_put_sync()// to disable the clock pm_runtime_get_sync()// to enable again and reconfigure sysc lost with softreset If needed I think softreset should be handled in the device enable path rather than a new API. Regards, Omar -- 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 5/5] OMAP: mailbox: use runtime pm for clk and sysc handling
Hi, On Thu, Dec 16, 2010 at 2:28 AM, Varadarajan, Charulatha ch...@ti.com wrote: On Thu, Dec 16, 2010 at 12:17, Omar Ramirez Luna omar.rami...@ti.com wrote: Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register. Based on the patch sent by Felipe Contreras: https://patchwork.kernel.org/patch/101662/ Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com --- arch/arm/mach-omap2/mailbox.c | 27 +-- 1 files changed, 5 insertions(+), 22 deletions(-) diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c index 40ddeca..f5f72ba 100644 --- a/arch/arm/mach-omap2/mailbox.c +++ b/arch/arm/mach-omap2/mailbox.c @@ -14,6 +14,7 @@ #include linux/err.h #include linux/platform_device.h #include linux/io.h +#include linux/pm_runtime.h #include plat/mailbox.h #include mach/irqs.h @@ -34,12 +35,8 @@ #define MAILBOX_IRQ_NOTFULL(m) (1 (2 * (m) + 1)) /* SYSCONFIG: register bit definition */ -#define AUTOIDLE (1 0) #define SOFTRESET (1 1) -#define SMARTIDLE (2 3) #define OMAP4_SOFTRESET (1 0) -#define OMAP4_NOIDLE (1 2) -#define OMAP4_SMARTIDLE (2 2) /* SYSSTATUS: register bit definition */ #define RESETDONE (1 0) Is this still required? Yes, mailbox uses the softreset every time it is requested (and that it has no current users at the time), it does it before configuring sysc, as opossed to what this patch does (where clk is enabled, sysc configured, and then softreset), but no harm was seen with this sequence. AFAIK, enabling/disabling hwmod handles prm reset but not softreset bit, that's why I left it there. Regards, Omar -- 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/5] OMAP: mailbox: build device using omap_device/omap_hwmod
On Thu, Dec 16, 2010 at 2:44 AM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Thu, Dec 16, 2010 at 02:08:11PM +0530, Varadarajan, Charulatha wrote:
+ oh = omap_hwmod_lookup(mailbox);
+ if (!oh) {
+ pr_err(%s: unable to find hwmod\n, __func__);
+ return;
+ }
+
+ od = omap_device_build(omap-mailbox, -1, oh,
+ NULL, 0,
+ mbox_latencies, ARRAY_SIZE(mbox_latencies),
+ 0);
+ if (!od) {
Check for IS_ERR(od).
+ pr_err(%s: could not build device\n, __func__);
If you have an API which returns errors, and you're bothering to print
something when an error occurs, it's often useful to print the returned
error code, so that people can find out _why_ the error occurred.
Agree.
Thanks,
Omar
--
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 5/5] OMAP: mailbox: use runtime pm for clk and sysc handling
Hi, On Thu, Dec 16, 2010 at 10:32 AM, Kanigeri, Hari h-kanige...@ti.com wrote: @@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox) l = mbox_read_reg(MAILBOX_REVISION); pr_debug(omap mailbox rev %d.%d\n, (l 0xf0) 4, (l 0x0f)); - if (cpu_is_omap44xx()) - l = OMAP4_SMARTIDLE; - else - l = SMARTIDLE | AUTOIDLE; - mbox_write_reg(l, MAILBOX_SYSCONFIG); - The OMAP4 mailbox sysconfig register bits are laid out differently from previous OMAP mailbox's. Example is smart idle bit location is different from previous OMAPs. Can I know as how are you handling this aspect in hwmod code ? hwmod framework provides definition for both IP models (omap_hwmod_sysc_type1for omap2/3 or omap_hwmod_sysc_type2 for omap4), these are located at arch/arm/mach-omap2/omap_hwmod_common_data.c Regards, Omar -- 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 13/14] OMAP2/3: PRCM: split OMAP2/3-specific PRCM code into OMAP2/3-specific files
On Tue, Dec 14, 2010 at 10:50 PM, Paul Walmsley p...@pwsan.com wrote:
Hi,
On Mon, 6 Dec 2010, Paul Walmsley wrote:
In preparation for adding OMAP4-specific PRCM accessor/mutator
functions, split the existing OMAP2/3 PRCM code into OMAP2/3-specific
files. Most of what was in mach-omap2/{cm,prm}.{c,h} has now been
moved into mach-omap2/{cm,prm}2xxx_3xxx.{c,h}, since it was
OMAP2xxx/3xxx-specific.
This process also requires the #includes in each of these files to be
changed to reference the new file name. As part of doing so, add some
comments into plat-omap/sram.c and plat-omap/mcbsp.c, which use
sideways includes, to indicate that these users of the PRM/CM includes
should not be doing so.
This patch has been updated to also take care of getting DSPBridge to
build again. Omar, Felipe, could you please take a look at the
mach-omap2/dsp.c and _tiomap.h changes and ack them?
...
diff --git a/arch/arm/mach-omap2/dsp.c b/arch/arm/mach-omap2/dsp.c
index 6feeeae..a8b62d7 100644
--- a/arch/arm/mach-omap2/dsp.c
+++ b/arch/arm/mach-omap2/dsp.c
@@ -9,11 +9,16 @@
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
+ *
+ * XXX The function pointers to the PRM/CM functions are incorrect and
+ * should be removed. No device driver should be changing PRM/CM bits
+ * directly; that's a layering violation -- those bits are the responsibility
+ * of the OMAP PM core code.
*/
#include linux/platform_device.h
-#include prm.h
-#include cm.h
+#include cm2xxx_3xxx.h
+#include prm2xxx_3xxx.h
#ifdef CONFIG_BRIDGE_DVFS
#include plat/omap-pm.h
#endif
I don't have a preference, I guess part of the license header makes
them even more noticeable.
diff --git a/drivers/staging/tidspbridge/core/_tiomap.h
b/drivers/staging/tidspbridge/core/_tiomap.h
index 1c1f157..7fac488 100644
--- a/drivers/staging/tidspbridge/core/_tiomap.h
+++ b/drivers/staging/tidspbridge/core/_tiomap.h
@@ -21,6 +21,12 @@
#include plat/powerdomain.h
#include plat/clockdomain.h
+/*
+ * XXX These mach-omap2/ includes are wrong and should be removed. No
+ * driver should read or write to PRM/CM registers directly; they
+ * should rely on OMAP core code to do this.
+ */
+#include mach-omap2/cm2xxx_3xxx.h
#include mach-omap2/prm-regbits-34xx.h
#include mach-omap2/cm-regbits-34xx.h
#include dspbridge/devdefs.h
Acked-by: Omar Ramirez Luna omar.rami...@ti.com
Just in case someone is wondering, there is a plan to use hwmod and
move start/stop/monitor functions to dsp.c code, so, the driver can
call them through pdata.
Regards,
Omar
--
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: omap_device: use pdev as parameter to get rt_va
Hi Benoit, On Mon, Dec 13, 2010 at 2:12 AM, Cousson, Benoit b-cous...@ti.com wrote: Hi Omar, On 12/11/2010 12:45 AM, Ramirez Luna, Omar wrote: Make the parameter received by omap_device_get_mpu_rt_va consistent with the functions meant to be called by drivers. Also move its header declaration to appear in the set of functions to be used by drivers, as per the comment there. Please note that even if Paul submitted this API upon request from Santosh, we do not want driver to us it most of the time. Oh, ok. Yes, I was under the impression that this ioremap was internal to hwmod, and drivers should do their own one; but then I noticed that API and since it was under the public functions through struct platform data, I thought it was easier to pass pdev than od. All drivers should us the generic Linux way to get physical mem resource and then ioremap it. Then I guess this function belongs to the public for core code and not for drivers along with the omap_device_get_pwrdm. I assume that if you want to update this API, you are probably already using it. Why cannot you use the generic way? I can leave the generic way along with ioremap, the purpose was to use omap_device APIs as much as possible. Thanks, Omar -- 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: use the right type for list_is_last
On Sun, Dec 12, 2010 at 6:02 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Omar, On Wednesday 08 December 2010 23:20:23 Omar Ramirez Luna wrote: Removes the following warning: CC [M] drivers/staging/tidspbridge/rmgr/rmm.o drivers/staging/tidspbridge/rmgr/rmm.c: In function 'rmm_alloc': drivers/staging/tidspbridge/rmgr/rmm.c:147: warning: passing argument 1 of 'list_is_last' from incompatible pointer type include/linux/list.h:170: note: expected 'const struct list_head *' but argument is of type 'struct rmm_ovly_sect *' I was about to send the same patch. Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com Great, thanks Laurent, Ionut for the review. Regards, Omar -- 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] staging: tidspbridge: rmgr/node.c code cleanup
Hi, On Sun, Dec 12, 2010 at 8:20 AM, Ionut Nicu ionut.n...@mindbit.ro wrote: Are the other modes (rdma, zerocopy) not supported at all? No, those modes are not supported... the only one working for dsp streams is proc-copy. Is there any plan to make them work? If not, maybe the code that handles them should be removed... I don't think there is any plan to support these modes, they have been disabled long time ago, even before the first versions of tidspbridge, I agree we should start thinking about removing them. Regards, Omar -- 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] staging: tidspbridge: rmgr/node.c code cleanup
Hi Ionut,
On Thu, Dec 9, 2010 at 4:22 PM, Ionut Nicu ionut.n...@mindbit.ro wrote:
Hi Omar,
On Thu, 2010-12-09 at 23:47 +0200, Ionut Nicu wrote:
Reorganized some code in rmgr/node.c to increase its
readability. Most of the changes reduce the code
indentation level and simplifiy the code. No functional
changes were done.
snip
/*
* Check stream mode. Default is STRMMODE_PROCCOPY.
*/
- if (!status pattrs) {
- if (pattrs-strm_mode != STRMMODE_PROCCOPY)
- status = -EPERM; /* illegal stream mode */
-
- }
- if (status)
- goto func_end;
+ if (pattrs pattrs-strm_mode != STRMMODE_PROCCOPY)
+ return -EPERM; /* illegal stream mode */
While cleaning up and reviewing the code I saw this check, which unless
it's intentional I think it looks like a bug. See comment below.
snip
+ strm_mode = pattrs ? pattrs-strm_mode : STRMMODE_PROCCOPY;
+ switch (strm_mode) {
+ case STRMMODE_RDMA:
...
+
+ case STRMMODE_ZEROCOPY:
...
+ case STRMMODE_PROCCOPY:
It looks like all modes are handled here although the only one that
could escape the validation from above is STRMMODE_PROCCOPY.
I tried removing the first check and tested with the
userspace-dspbridge strmcopy application, but I noticed the DSP hangs if
I try to use it with anything else than the copy mode.
Are the other modes (rdma, zerocopy) not supported at all?
No, those modes are not supported... the only one working for dsp
streams is proc-copy.
Regards,
Omar
--
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: remove file handling functions for loader
Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. Why doesn't the default firmware loading rule that comes with udev work properly for you? What are you needing different here that works properly for all other drivers? firmware.sh under /lib/udev/ and dsp binaries installed under /lib/firmware/, my rule is the brute version of firmware.sh so nothing different in the script. Probably the only change would be to supply the firmware name only, as of now the insmod parameter requires the entire path, e.g.: insmod bridgedriver.ko base_img=/lib/dsp/baseimage.dof if using firmware.sh and placing firmware files under /lib/firmware/, then insmod bridgedriver.ko base_img=baseimage.dof Should be enough. Regards, Omar -- 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: remove file handling functions for loader
On Wed, Dec 8, 2010 at 5:08 PM, Greg KH g...@kroah.com wrote: On Wed, Dec 08, 2010 at 05:02:20PM -0600, Ramirez Luna, Omar wrote: Hi, On Wed, Dec 8, 2010 at 4:26 PM, Greg KH g...@kroah.com wrote: On Tue, Dec 07, 2010 at 12:09:06AM -0600, Omar Ramirez Luna wrote: Instead use request_firmware and friends to get a valid firmware image. Right now the image is supplied dynamically through udev and the following rule: KERNEL==omap-dsp, SUBSYSTEM==firmware, ACTION==add, \ RUN+=/bin/sh -c 'echo 1 /sys/$DEVPATH/loading; \ cat $FIRMWARE /sys/$DEVPATH/data; \ echo 0 /sys/$DEVPATH/loading' Why do you need a custom firmware rule? It was meant as an example, when I compiled my minimal file system it didn't supply the firmware.sh script nor created /lib/firmware... I thought that not everybody would have the firmware.sh, so I just provided a sample rule. So, can I remove this from the changelog comment, as it's not really needed at all? Yes it can be removed. BTW, I don't expect this pushed through staging yet, I need to include it to my branch first and then I'll send a pull request with the pile of patches. Sorry for the misunderstanding and thanks for the review. insmod bridgedriver.ko base_img=baseimage.dof Ick, why use a module parameter name at all? Why is this special and different from all other firmware users? They don't have to manually specify a file name, the driver does that. The thing is that there are N number of firmwares, e.g.: There is the official and usable firmware to play with multimedia baseimage.dof But there are also minimal firmwares to just ping or swap buffers with the dsp, when you just want to play around with basic features. Please fix up the patch to not require a module parameter, distros hate them, and users hate them even more. The driver is the one requiring the parameter (it was already this way), this patch doesn't introduce any parameter. I'll check what can be done and if nobody rejects I'll use the baseimage.dof as firmware by default. Regards, Omar -- 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/6] OMAP: device: make rt_va easily avaialable to drivers
On Wed, Dec 8, 2010 at 5:54 PM, Omar Ramirez Luna omar.rami...@ti.com wrote:
Patch OMAP: hwmod/device: add omap_{device, hwmod}_get_mpu_rt_va[1],
introduces omap_device_get_rt_va which is meant to be called
by drivers to retrieve the _mpu_rt_va, however this function
receives a pointer to an omap_device; since there is no
practical way for a driver to get this parameter without
fiddling with pdev and container_of macro, and omap_device code
already does this, it is better for it to handle this case.
Also moved header declaration to appear in the set of
functions to be used by drivers, as per the comment there.
[1] http://marc.info/?l=linux-omapm=127808467703366w=2
Signed-off-by: Omar Ramirez Luna omar.rami...@ti.com
---
arch/arm/plat-omap/include/plat/omap_device.h | 3 +--
arch/arm/plat-omap/omap_device.c | 8 ++--
2 files changed, 7 insertions(+), 4 deletions(-)
This is a single patch set, please ignore the [1/6] subject :/
I'll resubmit to avoid confusions.
Regards,
Omar
--
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 8/8] staging: tidspbridge - make sync_wait_on_event interruptible
On Mon, Oct 25, 2010 at 5:17 PM, Guzman Lugo, Fernando fernando.l...@ti.com wrote: So that avoid nonkillable process. Signed-off-by: Fernando Guzman Lugo x0095...@ti.com --- .../staging/tidspbridge/include/dspbridge/sync.h | 13 +++-- 1 files changed, 11 insertions(+), 2 deletions(-) Pushed to dspbridge. Regards, Omar -- 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: overwrite DSP error codes
On Wed, Nov 3, 2010 at 7:31 PM, Rene Sapiens rene.sapi...@ti.com wrote: When calling the DSP's remote functions, the DSP returns error codes different from the ones managed by the kernel, the function's return value is shared with the MPU using a shared structure. This patch overwrites those error codes by kernel specifics and deletes unnecessary code. Signed-off-by: Rene Sapiens rene.sapi...@ti.com --- drivers/staging/tidspbridge/rmgr/disp.c | 44 +- 1 files changed, 8 insertions(+), 36 deletions(-) Pushed to dspbridge. Regards, Omar -- 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/3] staging: tidspbridge: fix mgr_enum_node_info
On Fri, Nov 5, 2010 at 12:01 PM, Ionut Nicu ionut.n...@gmail.com wrote: From: Felipe Contreras felipe.contre...@gmail.com The current code was always returning a non-zero status value to userspace applications when this ioctl was called. The error code was ENODATA, which isn't actually an error, it's always returned by dcd_enumerate_object() when it hits the end of list. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- drivers/staging/tidspbridge/rmgr/mgr.c | 5 + 1 files changed, 5 insertions(+), 0 deletions(-) Pushed to dspbridge. Regards, Omar -- 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 v3 0/3] staging: tidspbridge: bugfixes
On Fri, Nov 5, 2010 at 11:31 AM, Ionut Nicu ionut.n...@gmail.com wrote: Changes since v1: * Split the mgr_enum_node_info patch into two patches: one that fixes the issue and one that reorganizes the code. Ionut Nicu (3): staging: tidspbridge: fix mgr_enum_node_info staging: tidspbridge: mgr_enum_node_info cleanup staging: tidspbridge: fix kernel oops in bridge_io_get_proc_load Pushed to dspbridge. Regards, Omar -- 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 v3 00/12] staging: tidspbridge: various cleanups
Hi Ionut, On Sun, Nov 21, 2010 at 4:46 AM, Ionut Nicu ionut.n...@gmail.com wrote: This set of patches replaces some of the redundant components of the tidspbridge driver, such as: * wrapper functions for kmalloc/kfree * custom bitmap implementation * custom linked list implementation (list_head wrapper) with the standard linux kernel interfaces. The patches also do some code reorganization for increasing readability. Most of the changes reduce the code indentation level and simplify the code. No functional changes were done. There are many places in this driver that need this kind of cleanup, but these patches only fix the functions that were touched while converting the code to use linux bitmap and list_head. Thanks for your patches, I just fixed the style of some multiline comments before pushing, since these are very minor fixes I avoided the noise of resending the patches. Please remember to follow the Coding Style for multi line comments whenever you insert back those lines (even if the code was that way). Ionut Nicu (12): staging: tidspbridge: remove gs memory allocator staging: tidspbridge: remove utildefs staging: tidspbridge: switch to linux bitmap API staging: tidspbridge: remove gb bitmap implementation Pushed staging: tidspbridge: rmgr/node.c code cleanup Changes requested staging: tidspbridge: convert core to list_head Fixed a multiline style comment staging: tidspbridge: convert pmgr to list_head Fixed 2 multiline style comment staging: tidspbridge: convert rmgr to list_head staging: tidspbridge: remove custom linked list Pushed staging: tidspbridge: core code cleanup Fixed multiline style comments staging: tidspbridge: pmgr code cleanup Fixed multiline style comments staging: tidspbridge: rmgr code cleanup Doesn't apply because of patch staging: tidspbridge: rmgr/node.c code cleanup was not pushed. Regards, Omar -- 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
