RE: [PATCH v4] OMAP3: DSS: Kconfig changes to enable display options on OMAP3
On Tue, 2010-11-16 at 07:09 +0100, ext Nilofer, Samreen wrote: > Hi, > Any more comments on this patch? I'm not sure if enabling kernel options by default is a good thing. Somehow I remember that enabling things by default is not looked kindly at. Shouldn't they be enabled in the arch/arm/configs config file, if they are wanted? Tony, do you have opinnion on this? Should DSS features be enabled by default in the Kconfig files, or only in the board config file? Also, see one comment inline. > Warm Regards, > Samreen > Nilofer, Samreen wrote: > > The defconfig options for display are taken in the respective > > Kconfig to enable display by default on OMAP3 platforms > > > > Signed-off-by: Samreen > > --- > > Version4: > >Remove the enabling of the display panels by default. > > > > Version3: > >Eliminate the separate default number of FBs for > > different architecture. Keeping default FBs as 3 as before. > > > > Version2: > > Enables by default NEC panel used in zoom2/3/3630sdp, > > instead of Sharp LQ043T1DG01 panel enabled in previous > > version of this patch. > > > > drivers/video/omap2/dss/Kconfig|6 -- > > drivers/video/omap2/omapfb/Kconfig |1 + > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/video/omap2/dss/Kconfig > > b/drivers/video/omap2/dss/Kconfig index 43b6440..f3244a2 100644 > > --- a/drivers/video/omap2/dss/Kconfig > > +++ b/drivers/video/omap2/dss/Kconfig > > @@ -1,6 +1,7 @@ > > menuconfig OMAP2_DSS > > tristate "OMAP2/3 Display Subsystem support (EXPERIMENTAL)" > > depends on ARCH_OMAP2 || ARCH_OMAP3 > > + default y > > help > >OMAP2/3 Display Subsystem support. > > > > @@ -9,7 +10,7 @@ if OMAP2_DSS > > config OMAP2_VRAM_SIZE > > int "VRAM size (MB)" > > range 0 32 > > - default 0 > > + default 4 > > help > > The amount of SDRAM to reserve at boot time for video RAM use. > > This VRAM will be used by omapfb and other drivers > > that need @@ -102,7 +103,8 @@ config OMAP2_DSS_FAKE_VSYNC > > config OMAP2_DSS_MIN_FCK_PER_PCK > > int "Minimum FCK/PCK ratio (for scaling)" > > range 0 32 > > - default 0 > > + default 4 if ARCH_OMAP2 || ARCH_OMAP3 > > + default 0 if ARCH_OMAP4 I think having default ratio of 4 would mean that for example high res modes wouldn't work on beagle. They require high pixel clock, which requires 1/2 pck/lck ratio. > > help > > This can be used to adjust the minimum FCK/PCK ratio. > > > > diff --git a/drivers/video/omap2/omapfb/Kconfig > > b/drivers/video/omap2/omapfb/Kconfig > > index 65149b2..923bf48 100644 > > --- a/drivers/video/omap2/omapfb/Kconfig > > +++ b/drivers/video/omap2/omapfb/Kconfig > > @@ -1,6 +1,7 @@ > > menuconfig FB_OMAP2 > > tristate "OMAP2/3 frame buffer support (EXPERIMENTAL)" > > depends on FB && OMAP2_DSS > > + default y > > > > select OMAP2_VRAM > > select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] serial: omap-serial: Add support for kernel debugger
The kgdb invokes the poll_put_char and poll_get_char when communicating
with the host. This patch also changes the initialization order because the
kgdb will check at the very beginning, if there is a valid serial
driver.
Signed-off-by: Cosmin Cojocar
---
drivers/serial/Makefile |2 +-
drivers/serial/omap-serial.c | 38 --
2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index c570576..ad86113 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
+obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
obj-$(CONFIG_SERIAL_QE) += ucc_uart.o
obj-$(CONFIG_SERIAL_TIMBERDALE)+= timbuart.o
@@ -88,4 +89,3 @@ obj-$(CONFIG_SERIAL_ALTERA_JTAGUART) += altera_jtaguart.o
obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
obj-$(CONFIG_SERIAL_MRST_MAX3110) += mrst_max3110.o
obj-$(CONFIG_SERIAL_MFD_HSU) += mfd.o
-obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
diff --git a/drivers/serial/omap-serial.c b/drivers/serial/omap-serial.c
index 03a96db..df6ba03 100644
--- a/drivers/serial/omap-serial.c
+++ b/drivers/serial/omap-serial.c
@@ -866,14 +866,44 @@ serial_omap_type(struct uart_port *port)
return up->name;
}
+#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
+
+#ifdef CONFIG_CONSOLE_POLL
+static void serial_omap_poll_put_char(struct uart_port *port, unsigned char ch)
+{
+ struct uart_omap_port *up = (struct uart_omap_port *)port;
+ unsigned int status;
+
+ do {
+ status = serial_in(up, UART_LSR);
+ udelay(1);
+ } while ((status & BOTH_EMPTY) != BOTH_EMPTY);
+
+ serial_out(up, UART_TX, ch);
+}
+
+static int serial_omap_poll_get_char(struct uart_port *port)
+{
+ struct uart_omap_port *up = (struct uart_omap_port *)port;
+ unsigned int status;
+ int ch;
+
+ do {
+ status = serial_in(up, UART_LSR);
+ udelay(1);
+ } while ((status & UART_LSR_DR) != UART_LSR_DR);
+
+ ch = (int)serial_in(up, UART_RX);
+ return ch;
+}
+#endif
+
#ifdef CONFIG_SERIAL_OMAP_CONSOLE
static struct uart_omap_port *serial_omap_console_ports[4];
static struct uart_driver serial_omap_reg;
-#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
-
static inline void wait_for_xmitr(struct uart_omap_port *up)
{
unsigned int status, tmout = 1;
@@ -1022,6 +1052,10 @@ static struct uart_ops serial_omap_pops = {
.request_port = serial_omap_request_port,
.config_port= serial_omap_config_port,
.verify_port= serial_omap_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+ .poll_put_char = serial_omap_poll_put_char,
+ .poll_get_char = serial_omap_poll_get_char,
+#endif
};
static struct uart_driver serial_omap_reg = {
--
1.6.3.3
--
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: [linux-pm] [PATCH ver. 2] PM: add synchronous runtime interface for interrupt handlers
On Sat, 20 Nov 2010, Alan Stern wrote:
> On Sat, 20 Nov 2010, Rafael J. Wysocki wrote:
>
> > On Friday, November 19, 2010, Alan Stern wrote:
> > > This patch (as1431b) makes the synchronous runtime-PM interface
> > > suitable for use in interrupt handlers. Subsystems can call the new
> > > pm_runtime_irq_safe() function to tell the PM core that a device's
> > > runtime-PM callbacks should be invoked with interrupts disabled
> > > (runtime_suspend and runtime_resume callbacks will be invoked with the
> > > spinlock held as well). This permits the pm_runtime_get_sync() and
> > > pm_runtime_put_sync() routines to be called from within interrupt
> > > handlers.
> > >
> > > When a device is declared irq-safe in this way, the PM core increments
> > > the parent's usage count, so the parent will never be runtime
> > > suspended. This prevents difficult situations in which an irq-safe
> > > device can't resume because it is forced to wait for its non-irq-safe
> > > parent.
> > >
> > > Signed-off-by: Alan Stern
>
> > > --- usb-2.6.orig/drivers/base/power/runtime.c
> > > +++ usb-2.6/drivers/base/power/runtime.c
> > > @@ -223,11 +223,19 @@ static int rpm_idle(struct device *dev,
> > > callback = NULL;
> > >
> > > if (callback) {
> > > - spin_unlock_irq(&dev->power.lock);
> > > + if (dev->power.irq_safe) {
> > > + spin_unlock(&dev->power.lock);
> > >
> > > - callback(dev);
> > > + callback(dev);
> > >
> > > - spin_lock_irq(&dev->power.lock);
> > > + spin_lock(&dev->power.lock);
> > > + } else {
> > > + spin_unlock_irq(&dev->power.lock);
> > > +
> > > + callback(dev);
> > > +
> > > + spin_lock_irq(&dev->power.lock);
> > > + }
> > > }
> >
> > I didn't like this change before and I still don't like it. Quite frankly,
> > I'm
> > not sure I can convince Linus to pull it. :-)
> >
> > Why don't we simply execute the callback under the spinlock in the
> > IRQ safe case?
>
> Because it wouldn't work. The job of the runtime_idle callback is to
> call pm_runtime_suspend when needed. But if the callback runs under
> the spinlock then pm_runtime_suspend would hang when it tries to grab
> the lock.
>
> I don't think Linus will object to this. What he doesn't like is when
> some code drops a lock, reacquires it, and then behaves as though the
> lock had been held all along. That's not the case here; rpm_idle()
> does not depend on any state remaining unchanged across the callback.
One other thing I forgot to mention... If Linus doesn't like the way
the new code drops the spinlock and then reacquires it, then he must
also not like the existing code, which does the same thing. The only
difference lies in whether or not interrupts are re-enabled.
Alan Stern
--
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 ver. 2] PM: add synchronous runtime interface for interrupt handlers
On Sat, 20 Nov 2010, Rafael J. Wysocki wrote:
> On Friday, November 19, 2010, Alan Stern wrote:
> > This patch (as1431b) makes the synchronous runtime-PM interface
> > suitable for use in interrupt handlers. Subsystems can call the new
> > pm_runtime_irq_safe() function to tell the PM core that a device's
> > runtime-PM callbacks should be invoked with interrupts disabled
> > (runtime_suspend and runtime_resume callbacks will be invoked with the
> > spinlock held as well). This permits the pm_runtime_get_sync() and
> > pm_runtime_put_sync() routines to be called from within interrupt
> > handlers.
> >
> > When a device is declared irq-safe in this way, the PM core increments
> > the parent's usage count, so the parent will never be runtime
> > suspended. This prevents difficult situations in which an irq-safe
> > device can't resume because it is forced to wait for its non-irq-safe
> > parent.
> >
> > Signed-off-by: Alan Stern
> > --- usb-2.6.orig/drivers/base/power/runtime.c
> > +++ usb-2.6/drivers/base/power/runtime.c
> > @@ -223,11 +223,19 @@ static int rpm_idle(struct device *dev,
> > callback = NULL;
> >
> > if (callback) {
> > - spin_unlock_irq(&dev->power.lock);
> > + if (dev->power.irq_safe) {
> > + spin_unlock(&dev->power.lock);
> >
> > - callback(dev);
> > + callback(dev);
> >
> > - spin_lock_irq(&dev->power.lock);
> > + spin_lock(&dev->power.lock);
> > + } else {
> > + spin_unlock_irq(&dev->power.lock);
> > +
> > + callback(dev);
> > +
> > + spin_lock_irq(&dev->power.lock);
> > + }
> > }
>
> I didn't like this change before and I still don't like it. Quite frankly,
> I'm
> not sure I can convince Linus to pull it. :-)
>
> Why don't we simply execute the callback under the spinlock in the
> IRQ safe case?
Because it wouldn't work. The job of the runtime_idle callback is to
call pm_runtime_suspend when needed. But if the callback runs under
the spinlock then pm_runtime_suspend would hang when it tries to grab
the lock.
I don't think Linus will object to this. What he doesn't like is when
some code drops a lock, reacquires it, and then behaves as though the
lock had been held all along. That's not the case here; rpm_idle()
does not depend on any state remaining unchanged across the callback.
Alan Stern
--
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 5/5] OMAP: mailbox: add notification support for multiple readers
Felipe, On Sat, Nov 20, 2010 at 5:31 AM, Felipe Balbi wrote: > Hi Hari, > > On Fri, 2010-11-19 at 22:01 -0600, Kanigeri, Hari wrote: >> Of course :), profiling was done before releasing this code and no >> difference observed with or without blocking notifier. All the OMAP4 > > would you share some numbers ? Sure. On an average, 135 us for round trip message from A9 userspace Process to a thread running on M3 that handles this message. The test is with the entire IPC stack on both ends. Thank you, Best regards, Hari -- 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 ver. 2] PM: add synchronous runtime interface for interrupt handlers
On Friday, November 19, 2010, Alan Stern wrote:
> This patch (as1431b) makes the synchronous runtime-PM interface
> suitable for use in interrupt handlers. Subsystems can call the new
> pm_runtime_irq_safe() function to tell the PM core that a device's
> runtime-PM callbacks should be invoked with interrupts disabled
> (runtime_suspend and runtime_resume callbacks will be invoked with the
> spinlock held as well). This permits the pm_runtime_get_sync() and
> pm_runtime_put_sync() routines to be called from within interrupt
> handlers.
>
> When a device is declared irq-safe in this way, the PM core increments
> the parent's usage count, so the parent will never be runtime
> suspended. This prevents difficult situations in which an irq-safe
> device can't resume because it is forced to wait for its non-irq-safe
> parent.
>
> Signed-off-by: Alan Stern
>
> ---
>
> Index: usb-2.6/include/linux/pm.h
> ===
> --- usb-2.6.orig/include/linux/pm.h
> +++ usb-2.6/include/linux/pm.h
> @@ -486,6 +486,7 @@ struct dev_pm_info {
> unsigned intrun_wake:1;
> unsigned intruntime_auto:1;
> unsigned intno_callbacks:1;
> + unsigned intirq_safe:1;
> unsigned intuse_autosuspend:1;
> unsigned inttimer_autosuspends:1;
> enum rpm_requestrequest;
> Index: usb-2.6/include/linux/pm_runtime.h
> ===
> --- usb-2.6.orig/include/linux/pm_runtime.h
> +++ usb-2.6/include/linux/pm_runtime.h
> @@ -40,6 +40,7 @@ extern int pm_generic_runtime_idle(struc
> extern int pm_generic_runtime_suspend(struct device *dev);
> extern int pm_generic_runtime_resume(struct device *dev);
> extern void pm_runtime_no_callbacks(struct device *dev);
> +extern void pm_runtime_irq_safe(struct device *dev);
> extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
> extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
> extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
> @@ -123,6 +124,7 @@ static inline int pm_generic_runtime_idl
> static inline int pm_generic_runtime_suspend(struct device *dev) { return 0;
> }
> static inline int pm_generic_runtime_resume(struct device *dev) { return 0; }
> static inline void pm_runtime_no_callbacks(struct device *dev) {}
> +static inline void pm_runtime_irq_safe(struct device *dev) {}
>
> static inline void pm_runtime_mark_last_busy(struct device *dev) {}
> static inline void __pm_runtime_use_autosuspend(struct device *dev,
> Index: usb-2.6/drivers/base/power/runtime.c
> ===
> --- usb-2.6.orig/drivers/base/power/runtime.c
> +++ usb-2.6/drivers/base/power/runtime.c
> @@ -223,11 +223,19 @@ static int rpm_idle(struct device *dev,
> callback = NULL;
>
> if (callback) {
> - spin_unlock_irq(&dev->power.lock);
> + if (dev->power.irq_safe) {
> + spin_unlock(&dev->power.lock);
>
> - callback(dev);
> + callback(dev);
>
> - spin_lock_irq(&dev->power.lock);
> + spin_lock(&dev->power.lock);
> + } else {
> + spin_unlock_irq(&dev->power.lock);
> +
> + callback(dev);
> +
> + spin_lock_irq(&dev->power.lock);
> + }
> }
I didn't like this change before and I still don't like it. Quite frankly, I'm
not sure I can convince Linus to pull it. :-)
Why don't we simply execute the callback under the spinlock in the
IRQ safe case?
Rafael
--
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 v1] AM35xx: Craneboard: Add USB EHCI support
Hello.
On 19-11-2010 19:07, srin...@mistralsolutions.com wrote:
From: Srinath
AM3517/05 Craneboard has one EHCI interface on board using port1.
GPIO35 is used as power enable.
GPIO38 is used as port1 PHY reset.
Signed-off-by: Srinath
---
arch/arm/mach-omap2/board-am3517crane.c | 21 +
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-am3517crane.c
b/arch/arm/mach-omap2/board-am3517crane.c
index 13ead33..0e1a806 100644
--- a/arch/arm/mach-omap2/board-am3517crane.c
+++ b/arch/arm/mach-omap2/board-am3517crane.c
[...]
@@ -53,10 +55,29 @@ static void __init am3517_crane_init_irq(void)
omap_gpio_init();
}
+static struct ehci_hcd_omap_platform_data ehci_pdata __initdata = {
+ .port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
+ .port_mode[1] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+ .port_mode[2] = EHCI_HCD_OMAP_MODE_UNKNOWN,
+
+ .phy_reset = true,
+ .reset_gpio_port[0] = 38,
+ .reset_gpio_port[1] = -EINVAL,
+ .reset_gpio_port[2] = -EINVAL
+};
+
static void __init am3517_crane_init(void)
{
omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
omap_serial_init();
+
+ /* Configure GPIO for EHCI port */
+ omap_mux_init_gpio(35, OMAP_PIN_OUTPUT);
+ gpio_request(35, "usb_ehci_enable");
+ gpio_direction_output(35, 1);
+ gpio_set_value(35, 1);
There's no need to call gpio_set_value(), as gpio_direction_output() has
already set the GPIO's value.
WBR, Sergei
--
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 5/5] OMAP: mailbox: add notification support for multiple readers
Hi Hari, On Fri, 2010-11-19 at 22:01 -0600, Kanigeri, Hari wrote: > Of course :), profiling was done before releasing this code and no > difference observed with or without blocking notifier. All the OMAP4 would you share some numbers ? > use cases are exercising this code. Just curious , are you doubting > the blocking notifier mechanism ? a little bit. Yeah. It added about 600ms of time spent on musb's probe when we were using blocking notifier for charger detection. When we moved to atomic notifier, that was solved. Anyway, you can never be sure when that will be scheduled and if cpu is really busy, it might pose a great deal. -- balbi -- 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 5/5] omap4: l2x0: Enable early BRESP bit
> -Original Message- > From: Måns Rullgård [mailto:m...@mansr.com] > Sent: Saturday, November 20, 2010 12:02 AM > To: Santosh Shilimkar > Cc: linux-omap@vger.kernel.org; n...@ti.com; m...@mansr.com; > t...@atomide.com; khil...@deeprootsystems.com; linux-arm- > ker...@lists.infradead.org > Subject: Re: [PATCH 5/5] omap4: l2x0: Enable early BRESP bit > > Santosh Shilimkar writes: > > > The AXI protocol specifies that the write response can only > > be sent back to an AXI master when the last write data has been > > accepted. This optimization enables the PL310 to send the write > > response of certain write transactions as soon as the store buffer > > accepts the write address. This behavior is not compatible with > > the AXI protocol and is disabled by default. You enable this > > optimization by setting the Early BRESP Enable bit in the > > Auxiliary Control Register (bit [30]). > > Did you measure the performance difference this makes, if any? > I didn't do any special runs for this bit alone. Just checked with hardware team and they confirmed that you would gain a bit on writes and it's good to enable it. -- 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 08/13] OMAP3: PM: Deny MPU idle while saving secure RAM
> -Original Message- > From: Nishanth Menon [mailto:n...@ti.com] > Sent: Saturday, November 20, 2010 2:45 AM > To: Kevin Hilman > Cc: Santosh Shilimkar; linux-omap; Jean Pihet; Vishwanath Sripathy; Tony > Subject: Re: [PATCH 08/13] OMAP3: PM: Deny MPU idle while saving secure > RAM > > Kevin Hilman had written, on 11/19/2010 03:06 PM, the following: > > Nishanth Menon writes: > > > >> Kevin Hilman had written, on 11/19/2010 02:39 PM, the following: > >> [...] > >>> In addtion, the patch from Santosh needs to better describe what other > >>> problems it is solving, since it is clearly not fixing this particular > >>> secure mode entry. Therefore, there must be others that are also > doing > >>> WFI. That being said, instead of such a generic fix as is done by > >>> Santosh's patch, maybe we need a common secure-mode entry point which > >>> does the necessary ROM code prep. > >> Ideally speaking - save_secure_ram can hit latencies which are pretty > >> bad.. eventually this logic should be moved outside the current > >> boundaries in some manner - unfortunately, I cant at the moment think > >> of a sane mechanism to do that given various proprietary and > >> not-mainlined-but-public security drivers for OMAP3 out there > >> :(. IMHO, the responsibility of secure storage should be with secure > >> drivers, but, at the moment touching that topic is opening up a > >> pandora's box :( > > > > Hmm, so the complexity and mess is pushed into the OMAP PM core... > > > > /me no likey > /me neither :( > > > > This specific patch controls the clock domain from auto idling around > the secure ram save. Apologies on the confusion - but if the [1] > patch > is fixing it, you can help me understand how it does it. > >>> Now that I understand the clockdomain part, I'm seeing the problem > >>> differently. (side note: A better written changelog could have > avoided > >>> this confusion by being clear that it was *clockdomain* idle that was > >>> being added here and that it was in addition to the existing > powerdomain > >>> settings.) > >>> > >>> Technically, $SUBJECT patch could have replaced the set_next_pwrst > with > >>> the clkdm_deny_idle. IOW, setting the pwrdm next state to is > redundant > >>> if you clkdm_deny_idle. > >>> > >>> I think this is the key to the confusion: > >>> > >>> 1) clkdm_deny_idle() implies the powerdomain stays on > >>> 2) setting powerdomain to on, does NOT imply clkdm_deny_idle() > >>> > >>> Another way of saying it is that setting a powerdomain to on does not > >>> prevent it from going inactive. It only prevents retention or off- > mode. > >> Agreed and I apologize for the confusion caused by the commit message > >> - > >> will it be sufficient for the purpose of this series to change the > >> commit log to better describe the patch? - I will leave the power > >> domain control to Santosh's /Tero's series instead. > >> > >> Is this acceptable option? > > > > That is a minimum requirement, but... > > > > Based on the rest of this series, I am not at all comfortable with > > managing this directly in the idle path. The latencies you mentioned > > above are only part of the reason. I have been trying to remove this > Keep in mind that the latency is incurred by the default settings in > this series *only* for the very first off mode currently. > > > kind of device idle/PM management from the core idle path and I am not > > enthused about adding stuff back. > > > > I would much rather see a separate, secure-mode driver, which for > > starters only manages secure RAM. It doesn't have to manage all of > > security stuff, but it will make a clearer (and cleaner) > > separation between the idle path and secure RAM management. If > > implemented as a driver, it could be much more intelligent about > > its save/restore and can behave just like any other driver that has to > > manage context save/restore. If the concern is about trying to have a > > general purpose "secure driver", then just call it a secure RAM driver > > or something to be clear it has a small, targetted scope. > There are few other issues with this approach. secure ram save by itself > is just a function. it's trigger should ideally be not just one security > driver IMHO - there is AES, SHAM, and other ones that will need to > implement runtime pm, context save and restore hooks -> E.g. Dimitry's > series[1] is trying to introduce an opensource security driver solution > for OMAP - this is just a start - it will be some time before these > drivers are ready and merged to mainline followed by power management > enablement - do we want to keep omap3 broken while a fix is available > till then? > I read the thread again. Indeed the change log confused me as well. The original patch as such is ok then. I thought you want to prevent MPU PD not to transition and that you are achieving by not allowing MPU clock domain idle. Then your patch seems to be right fix. Regards Santosh -- To unsubscribe from
RE: [PATCH 08/13] OMAP3: PM: Deny MPU idle while saving secure RAM
> -Original Message- > From: Kevin Hilman [mailto:khil...@deeprootsystems.com] > Sent: Saturday, November 20, 2010 2:40 AM > To: Nishanth Menon > Cc: Santosh Shilimkar; linux-omap; Jean Pihet; Vishwanath Sripathy; Tony > Subject: Re: [PATCH 08/13] OMAP3: PM: Deny MPU idle while saving secure > RAM > > Nishanth Menon writes: > > > Kevin Hilman had written, on 11/19/2010 02:55 PM, the following: > > [...] > >> Now, based on what you say below, it seems like there is no way to > >> guarantee that SMIs don't do this, so I guess we have no choice but to > >> protect them all. > > In that way, I do like the patch from Santosh - with the relevant > > changes we will need to incorporate. Do keep in mind that SMI is a > > secure service - In theory, there could be(and to my knowledge, are) > > custom secure services to do all kind of other things that are not > > power management related[1] - > > All the more reason that this secure mode code should be moved out of > the core idle path into its own driver. > I also think this is better idea. -- 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/13] OMAP3: OFF mode fixes
> -Original Message- > From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > ow...@vger.kernel.org] On Behalf Of Nishanth Menon > Sent: Saturday, November 20, 2010 3:07 AM > To: Kevin Hilman > Cc: linux-omap; Jean Pihet; Vishwanath Sripathy; Tony > Subject: Re: [PATCH 00/13] OMAP3: OFF mode fixes > > Kevin Hilman had written, on 11/19/2010 03:20 PM, the following: > > >> Request for testing this series for comparison between master and this > >> series requested for additional platforms where available. > > > > I haven't yet been through the entire series, but some general comments > > to share before the weekend: > thanks for comments so far. I will wait for the complete series to be > reviewed before reposting a v2. > > > > > The secure mode code is growing in size and complexity, so I think it > > should be removed from pm34xx.c and moved into it's own file > > (secure3xxx.c ?) This will help keep pm34xx.c lean, and it can call > > into secure code as needed. We already have omap44xx-smc.S. I could make this generic. One problem with The secure APIs, they are consistent in all OMAP version. For example Secure SAR save has a different API and signature on OMAP3 and OMAP4. But this is something doable. Regards, Santosh -- 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
