Re: [RFC] possible removal of omap-serial
On Thu, 2014-03-20 at 18:52 -0500, Felipe Balbi wrote: Hi folks, I've been toying with the idea of removing drivers/tty/serial/omap-serial.c since that's, to put it bluntly, an ungly copy of 8250 driver. The original concern was wrt suspend/resume but I think it'd be a far better approach to implement runtime PM in 8250 and write a rather small 8250-omap.c glue (much like 8250-acorn.c or 8250-dw.c) just to get the OMAP-specific details out of the way. The question I have is: omap-serial.c calls the serial devnodes ttyO\d, instead of ttyS\d so removing omap-serial.c would have a direct impact in userland. I wonder if it's an acceptable regression considering we'd be able to reuse 8250 gaining proper Flow Control support, proper DMA support, years and years of bug-fixes, etc. What do you guys say ? I think it would be better even if we have to support calling it ttyO%d somehow. Alan -- 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/3] capebus moving omap_devices to mach-omap2
Fair enough. But there's no such thing a 'hotplug enumeration construct' in Linux yet, and a bus is the closest thing to it. It does take advantage of the nice way device code matches drivers and devices though. A bus is the wrong construct. You need something to add devices onto the busses. You can do that. The Intel SFI layer on phones for example enumerates devices through a firmware table set and creates them on the right actual physical bus not on their own fake one. It's not hotplug in the sense that the phone happens be a fixed configuration but it does support hotplug behaviour because the order of the drivers and enumeration is undefined (and both orders work). I'm afraid that having the I2C/SPI drivers doing the detection won't work. The capes can have arbitrary I2C/SPI devices (and even more weird components). There is no way to assure for example that the I2C device responding to address 'foo' in cape A is the same I2C device responding to the same address in cape B. your -detect() method should take care of that. There isn't some magical serial number in I²C devices that a -detect() method can read and the implementation of I²C is somewhat It doesn't matter. What you are basically talking about is cape layer - wtf is this - how do I plumb it - create device nodes with correct name for binding, address etc on the right bus i2c layer - ooh a new i2c device - probe as indicated by device name - attach correct driver Architecturally its possible you want to make some caps MFDs if they have their own bus heirarchies etc but generally I doubt it. Take a look at arch/x86/platform/mrst/mrst.c. It's a specific example of a platform which parses tables and attaches devices to the right physical bus in a manner they can be reliably probed even when the device has no sane autodetect. Alan -- 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/3] capebus moving omap_devices to mach-omap2
Further, it is critical to enable hardware vendors to avoid writing any code for which there are existing drivers. Which is why you don't want to create a new bus type for it. Capebus seems to me to provide this solution fairly well. I don't believe the SFI approach covers the most critical aspects of hotplug behaviour. I think you missed the point - it just an example of doing this not one I'd suggest using directly. your -detect() method should take care of that. There isn't some magical serial number in I²C devices that a -detect() method can read and the implementation of I²C is somewhat It doesn't matter. What you are basically talking about is cape layer - wtf is this - how do I plumb it - create device nodes with correct name for binding, address etc on the right bus i2c layer - ooh a new i2c device - probe as indicated by device name - attach correct driver Many of the devices cannot be probed. Look more closely at the code I pointed you at. I wonder if have a differing understanding of the word probe in this situation. In the Linux space the driver bindings call the matching probe function based upon the device structure. In Linux the probe method does not mean scan the bus and enumerate/detect the devices If an i²c bus is thrown a device which has an address and a matching name the only thing it will attempt to do is to call the probe method of the device driver matching that name on the given i²c address. In other words its a probe in the sense of I've been told there is one of your widgets here, please take a look not a probe in the sense of scan 255 i²c addresses and poke randomly at them SFI for example creates entries for things like type foo pressure sensor at 0x68 on bus 3 and the core kernel i²c code will only call the foo drivers probe method and only on bus 3 and only for address 0x68. In your case you want to use your DT fragments or any other descriptor format to do exactly the same. Not create a new bus but add a bunch of devices to the existing busses. It's like hot plugging a PCI card - you don't create a new PCI bus, you add a device to the existing bus. In the PCI case the device has properties that uniquely identify it from hardware level. In the i²c case the properties come from your DT fragment or similar. The rest is the same. I know I *am* the slow person in the room, but doesn't this approach require the code to be compiled into the kernel to support the devices ahead of time? While I think it might be reasonable to have hardware The specific implementation in SFI does but thats a property of SFI. I'm not trying to push SFI itself anywhere except over the edge of a very tall cliff. The point is that you can take a description of things like i²c devices and have then properly identified on the existing busses using the existing bus architecture just fine. developers provide DT fragments in their EEPROMs, there's no way to get them to submit code patches in order to get their hardware to work. They need to ship hardware that works with pre-existing software, since there will be hundreds of boards created by people with little to no previous Linux experience (akin to Arduino). I seem to be missing how that approach would get us there. That is what I assume and what I believe can be provided without inventing imaginary bus types. Alan -- 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/3] capebus moving omap_devices to mach-omap2
What they want, and what every user wants, is I plug this board in, and the driver make sure everything is loaded and ready. No, the end users don't want to see any of the implementation details of how the bitfile is transported; the driver can handle it. That doesn't necessarily make it a bus merely some kind of hotplug enumeration of devices. That should all work properly both for devices and busses with spi and i²c as the final bits needed for it got fixed some time ago. In an ideal world you don't want to be writing custom drivers for stuff. If your cape routes an i²c serial device to the existing system i²c busses then you want to just create an instance of any existing driver on the existing i²c bus not create a whole new layer of goop. It does need to do the plumbing and resource management for the plumbing but thats not the same as being a bus. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 20/24] SERIAL: core: add xmit buffer allocation callbacks
On Sat, 06 Oct 2012 13:45:47 +0100 Russell King rmk+ker...@arm.linux.org.uk wrote: This allows drivers (such as OMAP serial) to allocate and free their transmit buffers in a sane manner, rather than working around the buffer allocation provided by serial_core. I think this just illustrates how broken the serial_core layering is in that drivers can't treat it as a library but get constrained by it. Fine for now and actually probably a useful hook to begin removing the circ buffers for the kernel generic kfifo and other work that needs doing eventually. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 00/24] OMAP serial driver flow control fixes, and preparation for DMA engine conversion
On Sat, 6 Oct 2012 13:38:03 +0100 Russell King - ARM Linux li...@arm.linux.org.uk wrote: Hi, This series of patches fixes multiple flow control issues with the OMAP serial driver, and prepares the driver for DMA engine conversion. We require hardware assisted flow control to work properly for DMA support otherwise we have no way to properly pause the transmitter. All the generic parts Acked-by: Alan Cox a...@linux.intel.com -- 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 09/17] serial/8250: Limit the omap workarounds to omap1
On Mon, 10 Sep 2012 22:31:04 -0700
Tony Lindgren t...@atomide.com wrote:
These workarounds do not apply for CONFIG_ARCH_OMAP2PLUS at all,
so let's make it just CONFIG_ARCH_OMAP1.
This is needed to for ARM common zImage changes for
omap2+ to avoid including plat and mach headers.
Cc: Alan Cox a...@linux.intel.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: linux-ser...@vger.kernel.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/tty/serial/8250/8250.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c
b/drivers/tty/serial/8250/8250.c index 8123f78..5b3f2fe 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -2336,7 +2336,7 @@ serial8250_do_set_termios(struct uart_port
*port, struct ktermios *termios, serial_port_out(port, UART_EFR, efr);
}
-#ifdef CONFIG_ARCH_OMAP
+#ifdef CONFIG_ARCH_OMAP1
/* Workaround to enable 115200 baud on OMAP1510 internal
ports */ if (cpu_is_omap1510() is_omap_port(up)) {
if (baud == 115200) {
@@ -2426,7 +2426,7 @@ static unsigned int serial8250_port_size(struct
uart_8250_port *pt) {
if (pt-port.iotype == UPIO_AU)
return 0x1000;
-#ifdef CONFIG_ARCH_OMAP
+#ifdef CONFIG_ARCH_OMAP1
if (is_omap_port(pt))
return 0x16 pt-port.regshift;
#endif
Acked-by: Alan Cox a...@linux.intel.com
Even better would be if for other cases is_omap_port and friends
returned 0...
--
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 09/17] serial/8250: Limit the omap workarounds to omap1
Even better would be if for other cases is_omap_port and friends returned 0... Yes it seems that those macros could be moved from plat-omap/serial.h to live in drivers/tty/serial/8250/8250.h? Or do you have some better place in mind? I've not looked at it enough to decide if it's doable or not. I'm happy either way - both patches are progress the right way! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] serial: omap: always return IRQ_HANDLED
On Tue, 21 Aug 2012 12:15:44 +0300 Felipe Balbi ba...@ti.com wrote: Even if we enter our IRQ handler just to notice that the our device didn't generate the IRQ, that still means handling and IRQ, so let's return IRQ_HANDLED. That looks wrong - you'll defeat the stuck IRQ protection. If we didn't cause the IRQ then we are IRQ_NONE ? -- 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 0/4] Add ability to set defaultless network device MAC addresses to deterministic computed locally administered values
Why should Ubuntu, Fedora etc stink up their OSes with Panda-specific workarounds? And Panda is not the only device with this issue. Why should we crap all over the kernel for all these board specific problems ? Userspace code is at least pageable and generally less security critical. So your argument from that point of view is bunk. There are tons and tons of boards doing tons of horrible hacks. If we mangled the generic code for all of them the result would be a complete unmanagable pile of turd. The use of locally administered MAC addressing is policy. The helper belongs in userspace as it's clearly part of what udev is supposed to be doing via device notifications, instead of your custom mini kernel-udev hack which is what you've basically created. We've said no to lots of people (several a year). We've done so for good reasons. Most of them had more taste than your hack too (ok except the Pi which was even more broken) You need a udev rule, one single tiddly udev rule, and perhaps to expose a sysfs node somewhere if the required generation data is on the board. Hardly stinking up the userspace is it. That would also then fix any races with userspace trying to set the MAC, it would remove the need for the helper. It will avoid encoding ultra-crappy assumptions like To make use of this safely you also need to make sure that any drivers that may compete for the bus ordinal you are using (eg, mUSB and ehci in Panda case) are loaded in a deterministic order. What are you going to do when speeding up booting by parallelising more probes breaks this kind of garbage assumption ? To be honest if Fedora needs to deal with an army of craptastic devices whose vendors can't get a MAC address on the board then they probably need a single common change to ifup so that if you ifup an interface that has no MAC it generates a local one. Thats about 6 lines of userspace code in the config scripts. It's also probably a good default end user behaviour. And if you have a real MAC but it's not loaded into the device you can just shove it into the platform device. End of problem. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: serial_core: Add mechanism to identify port closure.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 9c4c05b..c176ff2 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1426,6 +1426,9 @@ static void uart_port_shutdown(struct tty_port *port) */ uport-ops-shutdown(uport); + if (uport-ops-set_wake) + uport-ops-set_wake(uport, false); + /* * Ensure that the IRQ handler isn't running on another CPU. */ @@ -1512,6 +1515,9 @@ static int uart_open(struct tty_struct *tty, struct file *filp) goto err_dec_count; } + if (uport-ops-set_wake) + uport-ops-set_wake(uport, true); + /* * Make sure the device is in D0 state. */ This is probably the right approach for now. In the ideal world we should probably use the struct device * and device power management but that is a big upheaval and would mean fixing lots of other stuff first. So this looks good to me - providing it doesn't break anything else. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
On Thu, 19 Apr 2012 20:00:12 +0530 Raja, Govindraj govindraj.r...@ti.com wrote: On Thu, Apr 19, 2012 at 12:38 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: The point is that wakeups should be enabled whenever driver is in use, and disabled when the driver is not in use. Which is the tty_port methods for initialisation and shutdown, which are mutex protected against each other and hang up. Not sure how the uart layer glue exposes that. Is it okay to read the port flags to identify the port state in driver whether the serial port shutdown ops is called from port_suspend or due to port closure. Since port_shutdown gets called even from uart_suspend_port That sounds like someone needs to fix the uart code to avoid muddling up suspend/resume/open/close then, or at least pass the needed information down. Don't rely on port-flags ideally, they may well go away in part as they cease to be needed by the lock changes. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: serial_core: Add mechanism to identify port closure.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 9c4c05b..0dc246d 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -1284,6 +1284,8 @@ static void uart_close(struct tty_struct *tty, struct file *filp) uart_wait_until_sent(tty, uport-timeout); } + state-uart_port-closing = true; So what are the locking rules for this - when is it valid and when can it be tested. This also still seems a hack to me - the core tty code doesn't have suspend/resume/open/close confused. The uart layer does, so really the uart layer needs untangling. I'm skeptical yet another magic state variable is the answer, particularly when the locking model for the variable appears undefined. Teach the uart core code to pass an extra argument indicating whether its really doing shutdown/open or merely doing suspend/resume. It's perfectly sensible that it tries to use the same helper for both, its broken that the called code cannot tell which. The parameter would also be a local variable which avoids all the locking questions. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: omap-serial: Keep the wakeup mechanism enabled by default
The point is that wakeups should be enabled whenever driver is in use, and disabled when the driver is not in use. Which is the tty_port methods for initialisation and shutdown, which are mutex protected against each other and hang up. Not sure how the uart layer glue exposes that. Alan -- 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/3] OMAP2+: UART: Enable tx wakeup + remove cpu checks
On Thu, 5 Apr 2012 16:54:03 +0530 Raja, Govindraj govindraj.r...@ti.com wrote: On Wed, Mar 21, 2012 at 3:54 PM, Govindraj.R govindraj.r...@ti.com wrote: From: Govindraj.R govindraj.r...@ti.com Based on Linux-OMAP tree uart branch. (git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git remotes/origin/uart) * Removes the cpu checks wherever possible and use version reg for populating features and errata's * enable tx wakeup available in wer reg for applicable module revision's Govindraj.R (3): OMAP2+: UART: Remove cpu checks for populating errata flags OMAP2+: UART: enable tx wakeup bit for wer reg OMAP2+: UART: replace omap34xx/omap4xx cpu checks with not omap24xx arch/arm/mach-omap2/serial.c | 13 + arch/arm/plat-omap/include/plat/omap-serial.h | 8 +++- drivers/tty/serial/omap-serial.c | 71 - 3 files changed, 78 insertions(+), 14 deletions(-) Alan / Greg, With you ack on tty parts can this series be upstreamed from omap tree ? Fine by me - this seems primarily to be architectural details. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 2/8] misc: ddr: add LPDDR2 data from JESD209-2
On Thu, 16 Feb 2012 15:57:57 +0530 Aneesh V ane...@ti.com wrote: On Thursday 16 February 2012 03:37 PM, Santosh Shilimkar wrote: On Saturday 04 February 2012 05:46 PM, Aneesh V wrote: add LPDDR2 data from the JEDEC spec JESD209-2. The data includes: 1. Addressing information for LPDDR2 memories of different densities and types(S2/S4) 2. AC timing data. This data will useful for memory controller device drivers I don't think it belongs in drivers/misc. It's not a driver but a library. lib/ might be a better place for it perhaps ? Alan -- 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/2] tty: serial: OMAP: work around broken driver, IP block
I will do it, although I do cling to the hope that others will help. I always work on the basis that if its had some coverage testing then the other folks who might have been affected should have reviewed the patch. They opted not to, within reason - their lookout. They'll only test it later when it his the tree if they don't do it earlier 8) Alan -- 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 serial device tree support
On Wed, 14 Dec 2011 07:20:13 -0800 Kevin Hilman khil...@ti.com wrote: Greg, Alan, Rajendra Nayak rna...@ti.com writes: v3 is rebased on top of the latest serial runtime patches[1] and boot tested with/without DT on OMAP4 SDP and OMAP4 Panda boards. With your ack on the drivers/tty/* stuff, I can queue this via the OMAP tree on top of the runtime PM conversion that it depends on. Go for 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 v8 00/20] OMAP2+: UART: Runtime adaptation + cleanup
On Tue, 06 Dec 2011 16:21:20 -0800 Kevin Hilman khil...@ti.com wrote: Govindraj.R govindraj.r...@ti.com writes: Converting uart driver to adapt to pm runtime API's. Code re-org + cleanup. Moving some functionality from serial.c to omap-serial.c Alan, can you confirm your Ack's are still valid on the drivers/tty parts of this series? It has gone through quite a few changes since your original ack. Yes -- 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/3] drivers/misc: introduce face detection module driver(fdif)
On Sat, 26 Nov 2011 12:31:44 +0800
tom.leim...@gmail.com wrote:
From: Ming Lei ming@canonical.com
One face detection IP[1] is integared inside OMAP4 SoC, so
introduce this driver to make face detection function work
on OMAP4 SoC.
This driver is platform independent, so in theory can
be used to drive same IP module on other platforms.
[1], ch9 of OMAP4 TRM
Signed-off-by: Ming Lei ming@canonical.com
If you are submitting it then it ought to have your sign off too. This
looks hardly ready for submission however.
+config FDIF
+ tristate Face Detection module
+ help
+ The FDIF is a face detection module, which can be integrated into
+ SoCs to detect the location of human beings' face in one image. At
+ least now, TI OMAP4 has the module inside.
So we have a completelt device specific API for what is becoming a more
general feature and on some hardware is tied to camera and the like. This
seems wrong
IMHO this should be part of video4linux because that'll make the
integrated stuff work sanely and for your cases it still provided the
right kind of mmap and format interfaces you need.
Detailed review below but basically this isn't ready to merge and it
shouldn't be merged from an interface point of view. Please work out how
to integrate the face detection into video4linux2 with the V4L folks.
+#define FDIF_DEV MKDEV(FDIF_MAJOR, 0)
+#define FDIF_MAX_MINORS 8
No custom majors please.
+static irqreturn_t handle_detection(int irq, void *__fdif);
Minor thing but you could just order the users right ...
+/*only support one fdif device now*/
+static struct fdif *g_fdif;
So why 8 minors ?
+ fdif-irq = platform_get_irq(pdev, 0);
+ if (fdif-irq 0) {
0 would also be a fail here surely - you can't support polling
+ dev_info(dev, fdif version=%8x hwinfo=%08x\n,
+ fdif_readl(fdif-base, FDIF_REVISION),
+ fdif_readl(fdif-base, FDIF_HWINFO));
Should be debug for productioncode
+ device_destroy(fdif_class, MKDEV(FDIF_MAJOR, 0));
+/*softreset fdif*/
+static int softreset_fdif(struct fdif *fdif)
+{
+ unsigned long conf;
+ int to = 0;
+
+ conf = fdif_readl(fdif-base, FDIF_SYSCONFIG);
+ conf |= SOFTRESET;
+ fdif_writel(fdif-base, FDIF_SYSCONFIG, conf);
+
+ while ((conf SOFTRESET) to++ 2000) {
+ conf = fdif_readl(fdif-base, FDIF_SYSCONFIG);
+ udelay(2);
+ }
+
+ if (to == 2000)
+ printk(KERN_ERR %s: reset failed\n, __func__);
dev_err
There are several that want fixing
+static irqreturn_t handle_detection(int irq, void *__fdif)
+{
+ unsigned long irqsts;
+ struct fdif *fdif = __fdif;
+
+ dump_fdif_regs(fdif, __func__);
+
+ /*clear irq status*/
+ irqsts = fdif_readl(fdif-base, FDIF_IRQSTATUS_j);
+ fdif_writel(fdif-base, FDIF_IRQSTATUS_j, irqsts);
+
+ if (irqsts FINISH_IRQ) {
+ read_faces(fdif);
Pity the rest of the code uses a mutex for locking and the IRQ code
doesn't bother with locking at all. That doesn't look safe to me.
+ } else if (irqsts ERR_IRQ) {
+ dev_err(fdif-pdev-dev, err irq!);
Not exactly user informative.
+ softreset_fdif(fdif);
+ } else {
This is only true if the IRQ is shared. Also the core IRQ code handles
spurious IRQ jams, and thirdly you shouldn't return HANDLED if you didn't
handle it.
+/*
+ * file operations
+ */
+static int fdif_open(struct inode *inode, struct file *file)
+{
+ struct fdif *fdif = g_fdif;
+ int ret = 0;
+
+ if (iminor(inode) || !fdif) {
+ printk(fdif: device is not correct!\n);
+ return -ENODEV;
+ }
So if its misconfigured or not found it would be a good idea to allow
users to spew stuff in the logs. Just return the error code.
The !fdif case here is meaningless. Either it matters in which case your
open isn't properly locked versus a remove, or it doesn't, in which case
your test is bogus.
+static int fdif_release(struct inode *inode, struct file *file)
+{
+ struct fdif *fdif = file-private_data;
+
+ if (!fdif)
+ return -ENODEV;
What if I unplugged the device while it was in use... ? What is your
locking model for all this.
+static ssize_t fdif_read(struct file *file, char __user *buf, size_t nbytes,
+loff_t *ppos)
+{
+ struct fdif *fdif = file-private_data;
+ ssize_t ret = 0;
+ unsigned len;
+ loff_t pos = *ppos;
+ int size;
+
+ /*not support unaligned read*/
+ if ((long)pos % sizeof(struct fdif_result))
+ return -EFAULT;
Wrong error code - EFAULT is a bad address in userspace really, and it'll
confuse folks if they get other stuff.
+ if (fdif-read_idx 0) {
+ ret = -EFAULT;
+ goto err;
Ditto
+ if (copy_to_user(buf, fdif-faces[fdif-read_idx], len)) {
+
Re: [PATCH 4/5] gpiolib: handle deferral probe error
On Fri, 07 Oct 2011 10:33:09 +0500 G, Manjunath Kondaiah manj...@ti.com wrote: The gpio library should return -EPROBE_DEFER in gpio_request if gpio driver is not ready. Why not use the perfectly good existing error codes we have for this ? We have EAGAIN and EUNATCH both of which look sensible. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tty: omap-serial: fix boot hang by converting to use a threaded IRQ handler (was Re: [PATCH] irq: always set IRQF_ONESHOT if no primary handler is specified)
On Mon, 22 Aug 2011 23:10:21 -0600 (MDT) Paul Walmsley p...@pwsan.com wrote: Convert the omap-serial hardirq handler to a threaded IRQ handler. Without this patch, OMAP boards which use the on-board OMAP UARTs and the omap-serial driver will not boot to userspace after commit f3637a5f2e2eb391ff5757bc83fb5de8f9726464 (irq: Always set IRQF_ONESHOT if no primary handler is specified). Enabling CONFIG_DEBUG_SHIRQ reveals 'IRQ handler type mismatch' errors: There are multiple other drivers reporting all these problems - the faulty irq change should be reverted at this point not the drivers fiddled with. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv5 0/7] HSI framework and drivers
On Fri, 10 Jun 2011 16:38:37 +0300 Carlos Chinea carlos.chi...@nokia.com wrote: Hi ! Here you have the fifth round of the HSI framework patches. Looks good to me - only other oddity I found is to build hsi_char on x86 you need slab.h before kmemleak.h or the build errors -- 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
OMAP2+: UART: Remove uart clock handling code from serial.c
On Fri, 29 Apr 2011 18:09:46 +0530 Govindraj.R govindraj.r...@ti.com wrote: Cleanup serial.c file in preparation to addition of runtime api's in omap-serial file. Remove all clock handling mechanism as this will be taken care with pm runtime api's in omap-serial.c file itself. 1.) Remove omap-device enable and disable. We can can use get_sync/put_sync api's 2.) Remove context save/restore can be done with runtime_resume callback for get_sync call. No need to save context as all reg details available in uart_port structure can be used for restore, so add missing regs in uart port struct. 3.) Add func to identify console uart. 4.) Erratum handling informed as flag to driver and func to handle erratum can be moved to omap-serial driver itself. Signed-off-by: Govindraj.R govindraj.r...@ti.com The tty parts of this look fine, the OMAP bits I have no idea. So for the tty bits with my serial maintainer hat on Acked-by: Alan Cox a...@linux.intel.com but its mostly up to the ARM/OMAP people -- 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: [GIT PULL] omap changes for v2.6.39 merge window
Absolutely. On Intel, it is (still) Windows the reference. If Windows doesn't boot on your motherboard you have a problem. So motherboard vendors won't make crazy incompatible things. They are constrained to OLPC, Moorestown ? fix their hardware because they just cannot alter Windows to suit their hardware differences. That really helps keeping actual differences to a minimum and only to things that are not fundamental. So Windows really helped making a uniform hardware platform on X86. That and the fact the Microsoft driver validation has driven a lot of standardisation along the we could write a driver and go through WHQL and ... and ... or we could just use a standard interface. Thing is though - the x86 platform does change, modern PC systems are very different to old ones (different IRQ controllers, different power management, different processor features, different bus interfaces, different firmware, ...) but someone bothered to make these *discoverable*. If I boot a Linux kernel on an AMD K6 I'm running with an 8259 interrupt controller, 8254/5 supporting I/O, a PC style keyboard controller, graphics via PCI or maybe AGP using memory on the card mostly, a one command at a time ATA interface based on WD1010 registers, APM based firmware that implements an extended version of the PC BIOS. If I boot it on a current PC I'm booting on a multiprocessor system with different timers, totally different IRQ controllers, different keyboard controllers (USB), PCI Express, an IOMMU, NCQ SATA, ACPI, graphics running in shared host memory able to give/take pages from the host, extra instructions, etc etc And the same kernel boots just fine on both just fine. -- 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: Handling multiple watchdogs
1) make sure that we have the new watchdog core infrastructure going in for 2.6.32. This new core integrates the common code that we use over and over again. I once wrote code for it and then Alan had different ideas and thoughts and wrote his updated code. I reviewed that and I am changing some small bits so that we will have the new What is the status of this ? I was thinking it had been a while but it seems to be 18 months ago.. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCHv4 4/7] HSI: hsi_char: Add HSI char device driver
+#define HSI_CHST_RD(c) ((c)-state HSI_CHST_RD_MASK)
+#define HSI_CHST_WR(c) ((c)-state HSI_CHST_WR_MASK)
+
+#define HSI_CHST_OC_SET(c, v) \
+ do { \
+ (c)-state = ~HSI_CHST_OC_MASK; \
+ (c)-state |= v; \
+ } while (0);
+
+#define HSI_CHST_RD_SET(c, v) \
+ do { \
+ (c)-state = ~HSI_CHST_RD_MASK; \
+ (c)-state |= v; \
+ } while (0);
+
+#define HSI_CHST_WR_SET(c, v) \
+ do { \
+ (c)-state = ~HSI_CHST_WR_MASK; \
+ (c)-state |= v; \
+ } while (0);
These sort of macros just hide detail - eg the lack of internal locking,
which can lead to problems later, plus they reference their arguments
multiple times so may have weird side effects.
They should probably be inline functions so they at least type check and
behave sanely, and their locking rules defintiely need documenting
+static long hsi_char_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct hsi_char_channel *channel = file-private_data;
+ unsigned int state;
+ struct hsi_config cfg;
+ struct hsc_rx_config rx_cfg;
+ struct hsc_tx_config tx_cfg;
+ long ret = 0;
+
+ if (HSI_CHST_OC(channel) != HSI_CHST_OPENED)
+ return -ENODEV;
-EIO is the traditional response in this case if the channel has been
closed by the other end - ENODEV indicates there is no physical device
present - not sure which is right here from the code.
+ } else if ((state == HSC_PM_ENABLE)
+ (channel-wlrefcnt 0)) {
+ ret = hsi_stop_tx(channel-cl);
+ if (!ret)
+ channel-wlrefcnt--;
What locks this lot against races (ditto some of the other ioctl code)
+ refcnt = atomic_inc_return(cl_data-refcnt);
+ if (refcnt == 1) {
You seem to construct a lot of clever stuff using atomic_inc_return and
friends where it looks like test_and_set_bit - or even a mutex over
open/close would be far easier to understand ?
+ ret = hsi_char_msgs_alloc(channel);
+
+ if (ret 0) {
+ refcnt = atomic_dec_return(cl_data-refcnt);
+ if (!refcnt)
+ hsi_release_port(channel-cl);
+ spin_lock_bh(channel-lock);
+ HSI_CHST_OC_SET(channel, HSI_CHST_CLOSED);
+ goto out;
+ }
+ if (refcnt == 1)
+ cl_data-attached = 1;
What happens here if a second open passes the first, the attached will
stay zero and other stuff will be in strange states but the open flag
will be set ?
+ for (i = 0; i HSI_CHAR_DEVS channels_map[i] = 0; i++) {
+ if (channels_map[i] = HSI_CHAR_DEVS) {
+ pr_err(Invalid HSI/SSI channel specified);
+ return -EINVAL;
+ }
+ set_bit(channels_map[i], ch_mask);
How will this integrate with hot discovery of SSI supporting devices ?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v6 02/12] media: Media device
On Thu, 25 Nov 2010 12:20:31 -0500 Andy Walls awa...@md.metrocast.net wrote: The signedness of char is ambiguous for 8 bit data, which is why an API would normally use u8 (or s8, I guess). Since this is known to be character data, I would think char would be fine. I am assuming C compilers would never assume multibyte chars. char is 8bit in all modern C. I have used C compilers with configurable 9 or 7 bit char and such a machine is never going to run Linux without serious insanity. Even grep in 9bit char is not fun... The advantage of using u8 is that any casting goes the way you expect whereas when working with UTF-8 things like int x = name[0]; may not produce the expected result otherwise. Alan -- 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 11/13 v3] serial: Add OMAP high-speed UART driver
On Mon, 27 Sep 2010 15:05:52 -0700 Kevin Hilman khil...@deeprootsystems.com wrote: Govindraj.R govindraj.r...@ti.com writes: This patch adds driver support for OMAP2/3/4 high speed UART. Cc'ing Greg KH and Alan Cox for review of the the new serial driver in this series. Greg/Alan, this has been on linux-serial several times now with review mostly from OMAP folks. If this looks OK from your side, with your ack(s) we can merge via the OMAP tree with the rest of the series which is a major reorg/cleanup of the OMAP platform init for serial drivers. Looks fine to me Alan -- 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: Sensors and the input layer (was Re: [RFC] [PATCH V2 1/2] input: CMA3000 Accelerometer driver)
1. Input transport via evdev is very convenient 2. There is no other standard alternative Once there is standard interface for such sensors they will happily use it and will not look back. I think the fact that most of the interest in IIO is how do we make an IIO/Input bridge speaks volumes. Sure, for a particular cell phone there is no ambiguity, the sensor has certain functionality assigned that is well known. But does this mean that we should not expect parts being reused at all anymore? If non-input uses later need non-input interfaces they can switch to that with an input bridge when there is one and when it happens, which probably won't. I am unsure how you would play a game with GPS as an input device. In a non-game context take a look at things like the British Museum application that allows you to view wherever you are and as it was long ago by fishing out a relevant photograph as you walk around. In a game context can I suggest the Zombies game is an example ? Alan -- 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: Sensors and the input layer (was Re: [RFC] [PATCH V2 1/2] input: CMA3000 Accelerometer driver)
IIO which is currently in staging. Except we had ALS before that as a layer and Linus vetoed it. So there is zero faith in IIO ever going anywhere. Instead we now have about ten different light sensor APIs to the point developers are writing a toolkit userspace plugin for *each* sensor. -- 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: Sensors and the input layer (was Re: [RFC] [PATCH V2 1/2] input: CMA3000 Accelerometer driver)
My hope is that we can make use of a well known and uniform API for all input devices in a device, be it a keypad, touchscreen, accelerometer, magnetometer, gyro, or whatever. If only we could agree what input devices are... Is that the right test for some of these devices. Surely the question is what devices can be meaningfully represented by the input API. The device range is always going to be quite large and people want to use the API because it means things just work. They can wire their home made surfboard unit and tilt sensors up to the PC and tuxracer just goes. They can wire pedals and a current meter to it and use it as the speed input in bzflag to simulate bicycle tanks etc.. Alan -- 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: Sensors and the input layer (was Re: [RFC] [PATCH V2 1/2] input: CMA3000 Accelerometer driver)
If non-input uses later need non-input interfaces they can switch to that with an input bridge when there is one and when it happens, which probably won't. Would there even be an argument which subsystem to use if IIO-input bridge existed today? Because if the answer is no then push into input is driven by convenience and not because it is the right solution. Probably because most of these devices have nothing to do with industrial I/O at all. If application does take something as an input it does not make it necessarily a human interface device. By this reasoning cameras should be represented as an input devices (why, some applications take input That's not what I asked. I really believe that input should represent purely human interface devices, not arbitrary data acquisition devices. That tends to make little sense where the API is the same and applications benefit enormously from consistency. I'd rather have an input-IIO bridge because that is the real world today ! The question is what does the API make *sense* for. Not what can you use the API for. Unix (and Linux) are enormously powerful because of the use of common interfaces and APIs. So a voltmeter really makes no sense. It's not a set of keys and it doesn't give X/Y/Z style readings. Nor does a camera. But a lot of things do fit this to varying degrees. I'm actually more dubious than Linus about ALS - because ALS tends not produce 'events' but to be sampled, and there are significant power implications to unnecessary polling. See it as a curse of success - because you got the API right and made it flexible people want to use it. And the more it's used the less special code is needed in user or kernel space for PDAs and phones - instead they just work. In a game context can I suggest the Zombies game is an example ? I am not familiar with this game, sorry. It uses GPS and networking to stage an 'in real world' zombie dodging game. Alan -- 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: Sensors and the input layer (was Re: [RFC] [PATCH V2 1/2] input: CMA3000 Accelerometer driver)
On Mon, 30 Aug 2010 11:04:39 -0500 Felipe Balbi m...@felipebalbi.com wrote: Hi Dmitry, When we tried to push N900's accelerometer driver as an input device you commented you didn't want sensors such as accelerometers, magnetometers, proximity, etc on the input layer because they are not user input, although I didn't fully agree with you, we had to modify the drivers and, I believe, one of them is sitting in staging under the industrial i/o subsystem. Are you now accepting sensor drivers on the input layer ? that will make our life a lot easier but we need some definition to avoid having to re-work drivers when we want to push them to mainline. I would certainly vote for them being input when they are sometimes used that way - compasses for example do get used by applications (like compass programs, some of the real cool visualisation tools and things like live/game mixed gaming environments) and accelerometers are gaming inputs. Proximity is also input for some stuff although usually of much more interest to the GUI manager than the GUI apps. ALS is more of a dual purpose thing -light levels are input features to the GUI on PDAs, although on many embedded devices they are most definitely part of the IIO subsystem. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 3/3] mm: iommu: The Virtual Contiguous Memory Manager
Why video4linux can't use the DMA API? Doing DMA with vmalloc'ed buffers is a thing that we should avoid (there are some exceptions like xfs though). Vmalloc is about the only API for creating virtually linear memory areas. The video stuff really needs that to avoid lots of horrible special cases when doing buffer processing and the like. Pretty much each driver using it has a pair of functions 'rvmalloc' and 'rvfree' so given a proper vmalloc_for_dma() type interface can easily be switched Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 3/3] mm: iommu: The Virtual Contiguous Memory Manager
On Tue, 13 Jul 2010 17:30:43 +0900 FUJITA Tomonori fujita.tomon...@lab.ntt.co.jp wrote: On Tue, 13 Jul 2010 09:20:12 +0100 Alan Cox a...@lxorguk.ukuu.org.uk wrote: Why video4linux can't use the DMA API? Doing DMA with vmalloc'ed buffers is a thing that we should avoid (there are some exceptions like xfs though). Vmalloc is about the only API for creating virtually linear memory areas. The video stuff really needs that to avoid lots of horrible special cases when doing buffer processing and the like. Pretty much each driver using it has a pair of functions 'rvmalloc' and 'rvfree' so given a proper vmalloc_for_dma() type interface can easily be switched We already have helper functions for DMA with vmap pages, flush_kernel_vmap_range and invalidate_kernel_vmap_range. I'm not sure they help at all because the DMA user for these pages isn't the video driver - it's the USB layer, and the USB layer isn't specifically aware it is being passed vmap pages. Alan -- 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] suspend blockers Android integration
On Sun, 6 Jun 2010 12:46:01 +0200 Florian Mickler flor...@mickler.org wrote: On Sun, 6 Jun 2010 12:00:47 +0200 Vitaly Wool vitalyw...@gmail.com wrote: Even worse, the suspend wakelock will keep the whole kernel active, as opposed to powering off unused devices separately as it's done in runtime PM. That is not true. While the kernel is not suspended it does runtime pm. On several of our platforms runtime PM already includes suspend so a suspend wakelock does interfere with existing power managemet at that level (not to mention the maintenance mess it causes). This is one of the reasons you want QoS information, it provides parameters by which the power management code can make a decision. Suspend blocksers simply don't have sufficient variety to manage the direction of power policy. If Android chooses to abuse the QoS information for crude suspend blocking then that is fine, it doesn't interfere with doing the job 'properly' on other systems or its use for realtime work on other boxes. Alan -- 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 0/8] Suspend block api (version 8)
[mtg: ] This has been a pain point for the PM_QOS implementation. They change the constrain back and forth at the transaction level of the i2c driver. The pm_qos code really wasn't made to deal with such hot path use, as each such change triggers a re-computation of what the aggregate qos request is. That should be trivial in the usual case because 99% of the time you can hot path the QoS entry changing is the latest one there have been no other changes If it is valid I can use the cached previous aggregate I cunningly saved in the top QoS entry when I computed the new one (ie most of the time from the kernel side you have a QoS stack) We've had a number of attempts at fixing this, but I think the proper fix is to bolt a disable C-states x interface into cpu_idle that bypases pm_qos altogether. Or, perhaps add a new pm_qos API that does the equivalent operation, overriding whatever constraint is active. We need some of this anyway for deep power saving because there is hardware which can't wake from soem states, which in turn means if that device is active we need to be above the state in question. -- 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 0/8] Suspend block api (version 8)
On Wed, 02 Jun 2010 11:10:51 +0200 Peter Zijlstra pet...@infradead.org wrote: On Wed, 2010-06-02 at 10:29 +0200, Thomas Gleixner wrote: If I understood you correctly then you can shutdown the CPU in idle completelty already, but that's not enough due to: 1) crappy applications keeping the cpu away from idle 2) timers firing Would solving those two issues be sufficient for you or am I missing something ? Aren't the container snapshot/resume people fighting the same set of problems here? And virtualisation - its a big one for virtualisation because if you've got 1000 misbehaving guests generating 100 wakeups a second you've got a problem Alan -- 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] misc : ROHM BH1780GLI Ambient light sensor Driver
Is there any standardisation of the ABIs whcih these drivers offer? If so, does this new driver comply with that? There was an attempt to sort this out but Linux vetoed it because he is under the delusion that light sensors are input devices. That doesn't work in many use cases as you have to go ask them the light level and the polling needed for input events keeps wrecks your idle behaviour when you need to sample them when required instead. Instead therefore the API is random and the devices appear in random ways and classes. We have some intel drivers to submit as well as and when sanity prevails. Alan -- 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 0/8] Suspend block api (version 8)
Android does not only run on phones. It is possible that no android devices have ACPI, but I don't know that for a fact. What I do know is that people want to run Android on x86 hardware and supporting suspend could be very benficial. Sufficently beneficial to justify putting all this stuff all over the kernel and apps ? That is a *very* high hurdle, doubly so when those vendors who have chosen to be part of the community are shipping phones and PDAs just fine without them. I would imagine the existing laptops will handle power management limited by the functionality they have available. Just like any other piece of hardware. I think existing laptops (and desktops) can benefit from opportunistic suspend support. If opportunistic suspend is used for auto-sleep after inactivity instead of forced suspend, the user space suspend blocker api will allow an application to delay this auto sleep until for instance a download completes. This part could also be done with a This assumes you modify all the applications. That isn't going to happen. The hardware is going to catch up anyway. alarms. I know my desktops can wakeup at a specific time by programming an RTC alarm, but without suspend blockers how do you ensure that the system does not suspend right after the alarm triggered? I have a system that wakes up at specific times requested How do you know that isn't the correct behavior. My laptop behaves in that way if for example the battery is almost flat. Your suspend blocker would cause me to lose all my work with a flat battery. This is another example of why the application must not be the policy manager. In the normal case in the PC world outside of corner cases like flat batteries the answer is really simple. The laptop suspend to RAM on idle intervals set in the BIOS and the like are sufficient that progress will have been made before it considers going back to sleep again. Right now its about ten seconds in each direction plus other costs (wear on LCD backlight, disc parking etc). Alan -- 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 0/8] Suspend block api (version 8)
Keep in mind, though, that a solution which is acceptable for Android has to include making sure that crappy applications don't cause the Ted if you are speaking for Android do you think you should post from a google address or with a google .sig ? battery to get drained. There seem to be some people who seem adamently against this requirement. From the Android folks' perspective, this is part of what is required to have an open app store, as opposed to one where each application has to be carefully screened and approved ala the Apple iPhone App Store. The other vendors appear to be managing nicely without magic blockers. I conjecture therefore there are other solutions. We need to agree on the requirements up front, because otherwise this is going to be a waste of everyone's time. This is why I am trying to understand the problem properly. And if we can't get agreement on requirements, I'd suggest appealing this whole thing to Linus. Either he'll agree to the requirements and/or the existing implementation, in which case we can move on with The existing implementation has been comprehensively rejected by half the x86 maintainers and scheduler people to start with. That's a fairly big 'No'. our lives, or he'll say no, in which case it will be blately obvious that it was Linux developer community who rejected the Android approach, despite a fairly large amount of effort trying to get something that satisfies *all* of the various LKML developers who have Ted save the politicing and blame mongering for management meetings please. If we don't have a solution it means that between us we couldn't find a viable solution. Maybe there isn't one, maybe we missed it. It's as much 'google rejects kernel approach' as 'kernel rejects google approach' and more importantly its actually 'we (cumulative) were not smart enough between us to figure it out' P.S. Keep in mind how this looks from an outsider's perspective; an embedded manufacturer spends a fairly large amount of time answering one set of review comments, and a few weeks later, more people pop up, and make a much deeper set of complaints, and request that the current implementation be completely thrown out and that someone new be designed from scratch --- and the new design isn't even going to meet all of the requirements that the embedded manufacturer thought were necessary. Is it any wonder a number of embedded developers have decided it's Just Too Hard to Work With the LKML? In some cases it is easier to do stuff yourself than work with others. One of the conditions of working in a public space is that you do so without harming others. This is why in much of the western world you can drive a car around your own land without a licence but must have one to drive on a public road. This is why a restuarant must meet different food standards to a home kitchen. This is why the kernel standards are higher than what you go off and do in private. Android is a very unique and extremely narrow environment. If it really is special enough to need its own kernel fork it isn't the first case for that and it's not a problem. The GPL is quite happy to encourage this. Time will then answer the questions because in 3 years time either every non Google phone will be kicking butt without suspend blockers, or every phone vendor using Linux with a traditional user space will be demanding them. Alan -- 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 0/8] Suspend block api (version 8)
Ted As a PS to the previous email the situation has I think more choices than you portray. Given the need for various constraints imposed by drivers for things like RT it's entirely possible that a solution ends up being something like Kernel proper: Turn suspend block kernel API into an expression of constraints (or whatever else seems to work) Throw the user space in the bin Google: Use the constraints in a sledgehammer manner (hey it solves your problem in that form so why not) Patch in a private user space API That makes things much much easier as we don't risk getting a horribly broken API into the kernel that is hard to remove, while hopefully meaning its rather easier for google to merge drivers and other code as well as to maintain a smaller patch set. -- 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 0/8] Suspend block api (version 8)
This is a much harder question to answer that what we need to use
opportunistic suspend. The question we ask is more like this: Is all
important work complete?. In the simplest case these can be the same,
I don't believe you can answer that question without telepathy and a
crystal ball.
The application doesn't know because it has no idea how to balance
conflicting resource demands or to infer the users requirements and
wishes. Most apps will misbehave anyway.
The OS doesn't know because it cannot tell what the app wants
So at best you have a heuristic.
What happens if the user presses the button right before you set QoS
of 'user apps' to QS_NONE?
Read down a paragraph.
To me it looks like this solution would result in this sequence which
may ignore the button press:
Button pushed
Button driver sets QoS of app it wakes to QS_ABOVESUSPEND
Set QoS of 'user apps' to QS_NONE
That would I think solve the reliable wakeup case although
drivers raising a QoS parameter is a bit unusual in the kernel.
That would at least however be specific to a few Android drivers
and maybe a tiny amount of shared driver stuff so probably not
unacceptable. (wake_up_pri(queue, priority); isn't going to
kill anyone is it - especially if it usually ignores the
priority argument)
Why is wake_up_pri(queue, priority) more acceptable than
suspend_block(...?
We keep it kernel side
It expresses policy and wishes rather than enforcing a behaviour.
What for example does suspend_block mean on a virtual machine ?
I would prefer priority was some kind of resource constraint model
instead but I'm just trying to think how to be absolutely minimally
invasible at this point.
What happens if the button press happend before this line:
count2 = tasks to QS_NONE | QS_NOTCHANGED
Screen off
Button Press
task to QS_ABOVESUSPEND
count = tasks that are QS_NOTCHANGED to QS_NONE
if (count != count2) {
Stuff happened ... rethink
}
That is still a bit weird and wonderful but all the logic is in the right
places. The special magic remains in the Android policy code and in the
kernel specifics for Android.
Thoughts ?
I don't think it works. Also, it does not seem much less invasive than
suspend blockers.
I don't think it works isn't that helpful. I don't think it works
because .. would help me a lot more.
I think it's a loss let invasive because you are not exposing a ton of
stuff to user space in general (just some private chit chat between a
couple of processes). I would prefer it didn't even do that but simply
used QoS guarantees. However I don't see a way to achieve that given your
intended QoS guarantees change according to actions like screen off.
Alan
--
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 0/8] Suspend block api (version 8)
My laptop behaves in that way if for example the battery is almost flat. Your suspend blocker would cause me to lose all my work with a flat battery. This is another example of why the application must not be the policy manager. You can still force suspend when the battery gets low. So now we need suspend blocker blockers - as I predicted. In the normal case in the PC world outside of corner cases like flat batteries the answer is really simple. The laptop suspend to RAM on idle intervals set in the BIOS and the like are sufficient that progress will have been made before it considers going back to sleep again. Right now its about ten seconds in each direction plus other costs (wear on LCD backlight, disc parking etc). I'm not sure what you are trying to say here. Are you saying your laptop enters S3 from idle? If I have an alarm set on my laptop it will wake up when the alarm goes off. Once it has woken up it will not go back to suspend (except for something libe a battery event) until a timeout has elapsed that began when the laptop woke up. This in the laptop work solves the problem of making progress. On a laptop power budget, with laptop constraints on suspend (both physical cycle limits of hardware and performance) this works fine. If I suspend/resume my laptop every time I have a 30 second idle gap I will need a new laptop much sooner than makes me happy. I don't claim this is true for a typical mobile phone obviously. Alan -- 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 0/8] Suspend block api (version 8)
Ok lets try and produce something more concrete. The control groups may be the wrong tool but we've got several such tools already Kernel involved acquire:mark myself important (into cgroup important) acquire(timeout)ditto, plus app timer/timeout handler release:mark myself unimportant (into cgroup downtrodden) All user isHeld: app implementation internal setReferenceCounted:app implementation internal In the idle manager [Androids own probably] if (member of ignored cgroup in user space) ignore for idle purposes In the Android code managing this [Android specific bits of probably userspace] mark downtrodden as ignored mark downtrodden as not ignored [Total kernel changes Ability to mark/unmark a scheduler control group as outside of some parts of idle consideration. Generically useful and localised. Group latency will do most jobs fine (Zygo is correct it can't solve his backup case elegantly I think) Test in the idling logic to distinguish the case and only needed for a single Android specific power module. Generically useful and localised] So I put my phone down The UI manager gets told the phone is 'down' Ten seconds later it is still down It marks the downtrodden group as 'ignored' The idle logic goes Nothing to run powersave Still nothing Ooh 0.3 seconds of nothing Drop into suspend state If I push the button we get an IRQ We come out of power save The app gets poked The app may be unimportant but the IRQ means we have a new timeout of some form to run down to idle The app marks itself important The app stays awake for 60 seconds rsyncing your email The app marks itself unimportant Time elapses We return to suspend If you are absolutely utterly paranoid about it you need the button driver to mark the task it wakes back as important rather than rely on time for response like everyone else. That specific bit is uggglly but worst case its just a google private patch to a few drivers. I understand why Android wants it. The narrower the gap between 'we are doing nothing, sit in lowest CPU on state' and 'we are off' the better the battery life and the more hittable the condition. Apart from that optional paranoia case my kernel now contains some trivial changes of generic value that have nothing to do with suspend blocking. Android has suspend blocking by choosing to use the generic features in its own specific way and we need almost no code writing ? Alan -- 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 0/8] Suspend block api (version 8)
On Fri, 28 May 2010 14:30:36 +0200 Peter Zijlstra pet...@infradead.org wrote: On Fri, 2010-05-28 at 13:21 +0100, Alan Cox wrote: [Total kernel changes Ability to mark/unmark a scheduler control group as outside of some parts of idle consideration. Generically useful and localised. Group latency will do most jobs fine (Zygo is correct it can't solve his backup case elegantly I think) Test in the idling logic to distinguish the case and only needed for a single Android specific power module. Generically useful and localised] I really don't like this.. Why can't we go with the previously suggested: make bad apps block on QoS resources or send SIGXCPU, SIGSTOP, SIGTERM and eventually SIGKILL Ok. Are you happy with the QoS being attached to a scheduler control group and the use of them to figure out what is what ? -- 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 0/8] Suspend block api (version 8)
The UI manager gets told the phone is 'down' Ten seconds later it is still down - wakeup event that should be delivered to untrusted app arrives here At this point you may mark the downtrodden group as ignored between the untrusted app receiving the event and the untrusted app marking itself as important. To avoid this you need the UI manager to receive every wakeup event in order to change its scheduling decisions. The event wakes the device, the event itself means the kernel is doing bits so the kernel is active and we are not idled so we have a time before we will consider re-suspending. [If you accept that untrusted apps must be constrained then you can't allow one to mark itself important - or at least you can't listen to it doing so and it goes to the static case which is trivial] (The cgroup has to have some awareness of suspend/resume so that it can allow the untrusted apps to be scheduled again) I don't think so. The apps will get scheduled anyway when not suspended. The only reason they are not being scheduled is that the device is suspended. Not just the button driver. Every driver that generates wakeupa. This gets difficult when it comes to the network layer, for instance, when the network driver has very little idea how the packet it just received will be routed. No. Every driver which generates wakeups which should wake an untrusted application. If network packets to untrusted applications should wake the box up then a simple background ping process left running is going to drain your battery and bugger your containment of the mess completely as you've just accepted an infinite supply of untrusted timed wakeup events with delay. Apart from that optional paranoia case my kernel now contains some trivial changes of generic value that have nothing to do with suspend blocking. Android has suspend blocking by choosing to use the generic features in its own specific way and we need almost no code writing ? The problem is that you still have a race, and fixing that race requires every event that could generate a wakeup to be proxied out to the policy manager as well. That's a moderate additional overhead. I am not convinced at this point. If the app gets put into the important group by the driver then you don't need to poke a policy manager. This again moves us beyond containment because we just allowed an 'untrusted' app a way to be trusted - just as it might abuse a suspend blocker. If you accept untrusted apps can't be fixed (for example they could simply lose the event internally due to app code bugs) then the static case all looks pretty trivial. With a Meego hat on you'd dump all the stuff you didn't trust into a scheduler group and tell the suspend aspect of the idle choice to ignore it when the screen blanks. While you are it you also get a free ticket to putting trusted rt apps into the 'and don't even C6' group. Alan -- 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 0/8] Suspend block api (version 8)
On Fri, 28 May 2010 12:41:23 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Fri, May 28, 2010 at 10:37:13AM +0100, Alan Cox wrote: The other vendors appear to be managing nicely without magic blockers. I conjecture therefore there are other solutions. Actually, no. A badly behaved application will kill the N900's battery life. Nobody else has managed nicely - they've just made life harder for application developers and users, which may have something to do with the relative levels of market adoption of Maemo and Android. I'm not aware of any form of resource management framework in MeeGo either, so as far as I know it'll have exactly the same problem. Maemo has battery management applications. Right now they show you what is going on but haven't gone to a pop-up 'XYZ is eating all your battery' kill it behaviour. The information is there. If my phone eventually becomes a 1GB RAM PC class system I will be running PC class apps on it and I will be migrating virtual machines to and from my phone which have no idea about the device properties of each device they migrate to and from. Be that as it may the question of how you manage a naughty app is a good one. Historically we've managed them for network abuse, memory abuse, cpu use abuse, access rights, but not yet power. Whether that looks like setrlimit(pid, LIMIT_CHARGE, 150mWH); or setrlimit(pid, LIMIT_POWER, 150mW); or something else is the question. I rather like the above but I don't see how to implement them nicely at the moment. Alan -- 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 0/8] Suspend block api (version 8)
I think you are missing the point. It works fine if the alarm caused the wakeup, but if you had just used your system and your inactivity timeout expired just as your alarm goes off, the alarm will not wake the system, nor does it prevent it from suspending. As far as I can tell (and its an extremely hard situation to replicate), this is not true. My laptop sleeps and wakes straight back up. The following cannot occur on my laptop for simple idling Alarm Suspend because the Alarm resets the suspend timer when it is delivered. The wake pins and wake logic also ensure that the sequence Suspend Alarm always causes Suspend Alarm Suspend Finishes Resume If I suspend/resume my laptop every time I have a 30 second idle gap I will need a new laptop much sooner than makes me happy. Then don't set your inactivity timeout to 30 seconds. I don't see how this is relevant. It's very relevant because it means that considering current laptops is not that important because they can't do this kind of fast sleep/wakeup I don't claim this is true for a typical mobile phone obviously. The only difference on the phone is that we have way more wakeup events which makes the race conditions more visible. The race exist on your laptop as well. The number of events is I think only partly relevant. What matters is how long you wait between idle and suspending. The longer you wait the less potential you have to end up with an event successfully owned by an application you are not considering relevant to suspend. -- 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 0/8] Suspend block api (version 8)
That's very good. But if it is done in a conceptually flawed way, some better solution should be considered for upstream merge. How is it flawed? Serious question. - It means changing drivers and quite a few apps - It doesn't solve the problem of rogue apps if they end up owning locks - It puts the deep knowledge of the platform in the applications - It gives the apps control of the action taken not policy indication - It doesn't resolve the problem of synchronization of take/releases stopping any suspend - The kernel parts are not generically useful, merely effective for solving a specific problem right now - even things like VM migration to/from phones seems to break it - It inverts the whole logic the kernel is following and trend it is following that suspend is simply a very deep idle (with implementations merged) If it was a localised turd I wouldn't worry. There are plenty df deep unmentionables hidden away enirely in platform specific code that deal with everything from stoned hardware engineers to crazed software stack implementations. Here is a question back the other way perhaps - If the existing kerne was almostl entirely read only, or you had to pay a large fee per line of code changed outside your own driver how would you implement the wakelock/suspend blocker API ? Because if you take the path that 'we want wakelockers' that is essentially the question you have to answer. How do you merge it so that nobody outside of your driver and maybe a spot of arch code knows about it. You are permitted a couple of sneaky substitions of core function bits in headers. Right now bits are going to leak out over the kernel which is the cause of friction. At the point it's invisible to everyone else they cease to be stakeholders so you don't have keep them happy. You've only got a couple in your patches but its painfully obvious from Matthew and your comments you'll end up needing a ton more and these will get everywhere as Android grows hardware platforms and CPU support as phones become more featureful and PC like. The moment a phone grows a USB base station with hub for example the entire USB stack becomes burdened with them. Matthew has already indicated networking needs them. Good luck with Dave Miller on that. I'm asking questions to look for generalised approaches, or even better doing it without new kernel stuff in the first place, but it's not the only way. Alan -- 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 0/8] Suspend block api (version 8)
I think the suggestion that has the closet fit with what we're trying to accomplish is Ingo's (or perhaps Ingo's explanation of Alan's): http://lkml.org/lkml/2010/5/28/106 where it's implemented as a constraint of some sort. I think we ended up in the same place on our own. Arve points out that qos constraint objects could work (but not if specifically tied to apps): http://lkml.org/lkml/2010/5/28/120 though he suggests that latency constraints don't represent this as well as state constraints. With latency you have an I don't give damn latency in your model which we could describe as infinite or manyana perhaps. Though if you look at it that way, then suspend_blockers become qos constraint objects, but their implementation and usage remain pretty much the same as we have now, which does not address Alan's concern regarding code turning up in drivers, etc. I'm not sure how you can solve this problem (avoiding races around entering/exiting the suspend or suspend-like state) without having a means for drivers to prevent entry to that state. I am much much less concerned about general expressions of constraint appearing in drivers. One of my early mails gave a list of other people/projects/problems that need them - from hard real time, to high speed serial on low end embedded to virtualisation. They fix a general problem in terms of a driver specific item. We end up making changes around the tree but we make everyone happy not just Android. Also we are isolating policy properly. The apps and drivers say I have these needs, the power manager figures out how to meet them. Where it gets ugly is if you start trying to have drivers giving an app a guarantee which the app then magically has to know to dispose of. If you are prepared to exclude untrusted apps from perfectly reliable event reporting (ie from finger to application action) that doesn't seem to be a neccessity anyway. livelock/deadlock/priority-inversion style issues due to interaction between different processes in different groups. Priority inversion with the cgroup case is like synchronization effects with the suspend blockers - its a real ugly problem and one that is known to be hard to fix if you let it happen so I agree there. -- 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 0/8] Suspend block api (version 8)
On Fri, 28 May 2010 15:02:37 +0100 Ok, I think I've misunderstood you. You're actually saying that only applications that are trusted to behave well are allowed to receive wakeup events? Yes, that makes implementation significantly easier. If To receive them in a manner that they are permitted to defer a suspend. There is non reason why bouncing cows shouldn't get to see an event, but there is always the miniscule possibility that we choose to suspend as it gets the event. That to me seems fine. Our starting basis was - Bouncing cows is not trusted Android's reaction was - We reserve the right to suspend bouncing cows where it likes it or not The caveat becomes - Bouncing cows may get suspended then get an event when the phone wakes back up. So I might press Moo just before a suspend and get the noise when it resumes. Given the untrusted cows could respond to the event otherwise by blocking the suspend for as long as permitted with a suspend blocker or similar that seems no worse. In this case probably better [oof zap! as opposed to 60 seconds of 'event, no sorry got a cow to draw at 100% CPU') As the app is untrusted we can't assume they would get suspend blockers right even if they had any. You can still be nice to the cows app and when the phone is put down send it a 10 second warning via dbus or Android equivalents. Your trusted call handling app can still request (by QoS or big hammers) that the phone does not suspend even if the app goes idle (because you have a wakeup latency QoS) A naïve trusted app will behave according to power management idling to suspend and get stopped A naïve untrusted app that is doing sane things will spend most of its life asleep and behave. -- 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 0/8] Suspend block api (version 8)
I think Arve's concern was the representation of the I care, but only a little or just low enough to ensure threads must run level which is what suspend blockers would map to (low enough to ensure we shouldn't halt the world but not necessarily implying a hard latency constraint beyond that). That's why I suggested manyana (can't get accents for mañana in a define) or perhaps dreckly[1]. They are both words that mean at some point but in a very very vague and 'relax it'll happen eventually' sense. More importantly it's policy. It's a please meet this constraint guide to the PM layer - not a you must do as say even if its stupid. They fix a general problem in terms of a driver specific item. We end up making changes around the tree but we make everyone happy not just Android. Also we are isolating policy properly. The apps and drivers say I have these needs, the power manager figures out how to meet them. That makes sense -- and as I've mentioned elsewhere, we're really not super picky about naming -- if it turns out that wakelocks/suspendblockers were shorthand for request a qos constraint that ensures that threads are running, we'll be able to get things done just as well as we do now. Cool. I think they are or at least they are close enough that nobody will notice the join ;) Where it gets ugly is if you start trying to have drivers giving an app a guarantee which the app then magically has to know to dispose of. Yeah -- which is something we've avoided in the existing model with overlapping wakelocks during handoff between domains. I'm not sure avoided is the right description - its there in all its identical ugliness in wakelock magic If you treat QoS guarantees as a wakelock for your purposes (which is just fine, drivers and apps give you policy, you use it how you like) then you could write the paragraph below substituting the word 'guarantee' for 'wakelock' So in that sense the mess is the same because in both cases you are trying to suspend active tasks rather than asking the task to behave and then taking remedial action with offenders. - input service is select()ing on input devices - when select() returns it grabs a wakelock, reads events, passes them on, releases the wakelock - the event subsystem can then safely drop its should be running threads constraint as soon as the last event is read because it has no queues for userspace to drain, but the overlapping wakelock prevents the system from immediately snapping back to sleep The conventional PC model is 'we don't go back into sleep proper fast enough for that race to occur'. It's hard to see how you change it. An app-device thank you for that event, I enjoyed it very much and have finished with it message moves the underlying event management and QoS knowledge into he driver proper but doesn't really change the interface. If you are prepared to exclude untrusted apps from perfectly reliable event reporting (ie from finger to application action) that doesn't seem to be a neccessity anyway. Currently in the Android userpace only trusted (system) apps can directly obtain wakelocks -- arbitrary apps obtain them via rpc to a trusted system service (which ensures the app has been granted permission to do this and tracks usage for accountability to user/developer). Clearly that would continue to work out. Alan [1] Dreckly being used in Cornwall, as one friend put it 'Like manãna but without that dreadful sense of urgency' -- 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 0/8] Suspend block api (version 8)
Now, if the user is playing this game, you want it to be scheduled. If the user has put down their phone and the screen lock has kicked in, you don't want it to be scheduled. So we could imagine some sort of cgroup that contains untrusted tasks - when the session is active we set a flag I would hope not, because I'd rather prefer my app that used the screen to get the chance to save important data on what it was doing irrespective of the screen blank: I have an elegant proof for this problem but my battery has gone flat (and I imagine we can play that examples game forever given Ashby's law) You can't express that with resource limits or QoS constraints. I don't see why not. You just have to think about the problem from the right end. Start from normality is well behaved applications and progress to but I can constrain bogus ones. So what are the resource constraints/QoS constraints for your example: [Simplistically] 1. App says 'I want to wakeup from events for me within 1 second' (Because I like drawing cows at about that rate) 2. App open driver for buttons 3. App opens driver for screen Driver for buttons goes 'humm, well I can trigger wakeup from all power states so I need no restrictions'. Screen will vary by device a lot. (I'll come back to the screen a bit more in a moment) So lets consider the same binary App runs on OLPC like h/w The pm code goes 'well I can suspend/resume in a second thats cool' The screen code goes 'Hey I've got OLPC like video so thats ok' The button driver can wake the system from suspend and queue an event App runs on Android like h/w The pm code goes 'well I can suspend/resume in a second thats cool' The screen code goes 'Gee the screen goes blank if I go below level X' so I'll set a limit The button driver can wake the system from suspend and queue an event App runs on Android like h/w but not trusted The pm code goes 'well tough, you can't do that, I'll refuse you' (Maybe user space wrapped by Android with a 'Cows wants to eat your phone alive [Refuse] [This Time Only] [Always] UI User hits refuse and Android duly assigns the code no guarantee and a hard limit of no guarantee. The screen code goes 'tough' The button driver can wake the system etc Cows will get suspended for longer than one second whether it likes it or not App runs on a desktop PC The pm code goes 'well I can't do suspend/resume in 1 second, but I can do C6' The screen goes 'I need C6' The button driver goes 'I need C6' Same app in each case and a lot less syscalls. No pathalogical cases either - with suspend blockers you can get emergent synchronization patterns between applications where each app rarely blocks suspends but together their timings fall such that they never allow it. How do you propose to even detect that ? Ok now the screen If I write to an X server and the server doesn't wish to talk to me it ignores the stream and I block. This has been the case since the 1980s. What is the problem here - your device driver for the display can block tasks it doesn't want to use the display. -- 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 0/8] Suspend block api (version 8)
Heck, for all I care, simply SIGKILL the thing and report it once the user starts looking at his screen again. Provide incentive for Joe Clicker to improve his app, instead of cope with the shit he created. That isn't helpful. But if you feel like that I suggest you run with your memory management protection disabled, it's really on there to deal with crap code and its giving the wrong incentives. Come to think of it you might want to remove your seatbelts and any safety catches or airbags - it only encourages carelessness. The reality is you need a sane, generic, race free way to express your requirements (eg for hard RT) and ditto for constraining the expression (for 'crapplications') Arguing that you don't need to do this isn't useful. Android has demonstrated a need to do this. RT has a need to do some of this. Virtualisation wants elements of this etc. The question is how you do it right. -- 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 0/8] Suspend block api (version 8)
The proposal I made a couple of days ago removes this API and leaves the other things (i.e., the in-kernel suspend blockers and opportunistic suspend) intact. In place of the userspace kernel-blocker API, Android would have to implement a power manager process that would essentially juggle all the latency requirements in userspace. On the kernel side you really want to add two arguments from day one which is the time in ms and the power state. We have enumerations of current power states (plus add suspend) so this is easy to do. The governors may not use them for a while but ACPI for example can use the latency pretty fast. You want the information there from day one so it is captured otherwise it ends up a right royal pain in the arse adjusting the API later. If the numbers are in but don't always get used it'll be a lot smoother. Communication between the power manager process and the kernel would be limited to adding a new opportunistic entry to /sys/power/state -- something which could well be useful in its own right. Even if this API turns out not to be good, it's simple enough that it could be removed pretty easily. Yes. Probably it really should be linked to a cpufreq driver thing - cpufreq_android or whatever being the starting point but thats not a big deal and it doesn't leak into all the apps either. This is detail however. Indeed, having a power manager thread may well turn out to be a useful thing -- but even if it doesn't, this scheme minimizes the damage while still allowing the Android platform to use a vanilla kernel with only limited modifications to their userspace. There have been some horribly bad attempts to do this, but if it works for Android and its encapsulated in cpufreq_android.c maybe with a private interface to a single supporting daemon then it's not creating bad user APIs, its not peeing in anyone elses pond and the policy will all be in the kernel and a daemon which keeps the wrong platform knowledge out of the application space. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 17:09:16 +0200 Peter Zijlstra pet...@infradead.org wrote: On Thu, 2010-05-27 at 11:06 -0400, Alan Stern wrote: Opportunistic suspends are okay. The proposed userspace API is too Android-specific. I would argue opportunistic suspends are not ok, and therefore the proposed API is utterly irrelevant. Assuming you are happy that opportunistically entering C6 and the like is ok via ACPI can you explain why you have a problem with opportunistic suspend and why it is different to a very deep sleep CPU state such as we have now (and which on many embedded platforms we deal with *is* sleeping external devices too) Alan -- 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 0/8] Suspend block api (version 8)
No. Suspend blockers are designed to ensure that suspend isn't racy with respect to wakeup events. The bit that mitigates badly written applications is the bit where the scheduler doesn't run any more. How does having applications taking blockers fix that - it makes it worse. They are now blocking the kernel doing the right job. If you're happy with a single badly written application being able to cripple your power management story, you don't need opportunistic And taking a suspend blocker isn't crippling your power management ??? What /is/ the correct way to solve this problem when entering explicit suspend states? How do you guarantee that an event has been delivered to userspace before transitioning into suspend? Now, this is a less interesting problem if you're not using opportunistic suspend, but it's still a problem. The same way as we deal with low power states on other non PC devices ? That's no good. If the input device has been deactivated, how does the wakeup event get delivered to the application? If the input device is letting itself get de-activated in a way that can lose events the input device driver is buggy. It's nobody elses business how it does the its job, and certainly *not* the applications. That's a kernel internal issue. You know the resource constraint exists because the driver knows it is open Your QoS guarantees tell you what you desired latency of response at the point you can become ready is. That's all your need to do it right. In kernel yes your device driver probably does need to say things like 'Don't go below C6 for a moment' just as a high speed serial port might want to say 'Nothing over 10mS please' I can't speak for Thomas, but I'm certainly not arguing that you don't need something that looks more like the blocker side of the logic *in kernel*, because there is stuff that you want to express which isn't tied to the task. So you need Userspace - QoS guarantee expression, implied resource expression via device use. *NO* knowledge of device or platform in the application Kernel space Drivers - Explicit guarantee expression not bound to tasks. Driver encapsulates the variety in the device hardware and expresses it in a uniform manner to the idling/suspend logic CPU Freq - Encapsulates the variety in the CPU and core power functionality of devices, makes policy based upon the uniform express from the drivers and tasks All the autonomy is now in the right places, and we have requisite variety to actually manage the situation. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 17:07:14 +0100
Matthew Garrett mj...@srcf.ucam.org wrote:
On Thu, May 27, 2010 at 04:05:43PM +0100, Alan Cox wrote:
Now, if the user is playing this game, you want it to be scheduled. If
the user has put down their phone and the screen lock has kicked in, you
don't want it to be scheduled. So we could imagine some sort of cgroup
that contains untrusted tasks - when the session is active we set a flag
I would hope not, because I'd rather prefer my app that used the screen
to get the chance to save important data on what it was doing
irrespective of the screen blank: I have an elegant proof for this
problem but my battery has gone flat
Perhaps set after callbacks are made. But given that the approach
doesn't work anyway...
Which approach doesn't work, and why ?
What is the problem here - your device driver for the display can block
tasks it doesn't want to use the display.
It's still racy. Going back to my example without any of the suspend
blocking code, but using a network socket rather than an input device:
int input = socket(AF_INET, SOCK_STREAM|SOCK_NONBLOCK, 0);
char foo;
struct sockaddr addr;
connect (input, addr, sizeof(addr))
while (1) {
if (read(input, foo, 1) 0) {
(do something)
} else {
(draw bouncing cows and clouds and tractor beams briefly)
}
}
A network packet arrives while we're drawing. Before we finish drawing,
the policy timeout expires and the screen turns off.
Which is correct for a badly behaved application. You said you wanted to
constrain it. You've done so. Now I am not sure why such a timeout
would expire in the example as the task is clearly busy when drawing, or
is talking to someone else who is in turn busy. Someone somewhere is
actually drawing be it a driver or app code.
For a well behaved application you are drawing so you are running
drawing stuff so why would you suspend. The app has said it has a
latency constraint that suspend cannot meet, or has a device open that
cannot meet the constraints in suspend.
You also have the socket open so you can meaningfully extract resource
constraint information from that fact.
See it's not the read() that matters, it's the connect and the close.
If your policy for a well behaved application is 'thou shalt not
suspend in a way that breaks its networking' then for a well behaving app
once I connect the socket we cannot suspend that app until such point as
the app closes the socket. At any other point we will break the
connection. Whether that is desirable is a policy question and you get to
pick how much you choose to trust an app and how you interpret the
information in your cpufreq and suspend drivers.
If you have wake-on-lan then the network stack might be smarter and
choose to express itself as
'the constraint is C6 unless the input queue is empty in which
case suspend is ok as I have WoL and my network routing is such
that I can prove that interface will be used'
In truth I doubt much hardware can make such an inference but some phones
probably can. On the other hand for /dev/input/foo you can make the
inference very nicely thank you.
Again wake on lan information does not belong in the application !
--
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 0/8] Suspend block api (version 8)
Opportunistic suspend is just a deep idle state, nothing else. No. The useful property of opportunistic suspend is that nothing gets scheduled. That's fundamentally different to a deep idle state. Nothing gets scheduled in a deep idle state either - its idle. We leave the idle state to schedule anything. I believe the constraint is - Do not auto-enter a state for which you cannot maintain the devices in use properly. On a current PC that generally means 'not suspend', on a lot of embedded boards (including Android phones) it includes an opportunistic 'suspend' and also several states half way between the PC deepest idles and suspend. Stop thinking about suspend as a special mechanism. It's not - except for s2disk, which is an entirely different beast. On PCs, suspend has more in common with s2disk than it does C states. Todays PCs are a special case. More to the point I don't think anyone is expected opportunistic suspend to be useful on _todays_ x86 systems. Even on todays PCs your assumption is questionable for virtual machines where a VM suspend is a lot faster and rather useful. Alan -- 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 0/8] Suspend block api (version 8)
What is a Correctly implemented driver in this case? One that receives a wakeup event and then prevents suspend being entered until userspace has acknowledged that event? Because that's what an in-kernel suspend blocker is. Kernel side maybe - but even then its a subset of expressing latency/lowest level requirements. That bit isn't really too contentious. You need a kernel object to hang a constraint off. ACPI provides no guarantees about what level of hardware functionality remains during S3. You don't have any useful ability to determine which events will generate wakeups. And from a purely practical point of view, since the latency is in the range of seconds, you'll never have a low enough wakeup rate to hit it. So PCs with current ACPI don't get opportunistic suspend capability. It probably won't be supported on the Commodore Amiga either - your point ? Suspend blockers are the mechanism for the driver to indicate whether the wakeup event has been handled. That's what they're there for. The in-kernel ones don't paper over anything. Semantically the in kernel blockers and the in kernel expression of device driven constraints are the same thing except that instead of yes/no you replace the boolean with information. So we go from block_suspend() / unblock_suspend() to add_pm_constraint(latency, level) remove_pm_constraint(latency, level); And if Android choses to interpret that in its policy code as if (latency MAGIC) suspend_is_cool(); else suspend_isnt_cool(); that's now isolated in droidspace policy Alan -- 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 0/8] Suspend block api (version 8)
network packet arrives. The difference between these scenarios is large.
Your application seems to change desired outcome every email. Sorry but
you need to explicitly explain and define a policy in full that we can
test ideas against. Otherwise this is useless.
If you have wake-on-lan then the network stack might be smarter and
choose to express itself as
'the constraint is C6 unless the input queue is empty in which
case suspend is ok as I have WoL and my network routing is such
that I can prove that interface will be used'
This is still racy. Going back to this:
int input = socket(AF_INET, SOCK_STREAM|SOCK_NONBLOCK, 0);
char foo;
struct sockaddr addr;
connect (input, addr, sizeof(addr))
while (1) {
if (read(input, foo, 1) 0) {
(do something)
} else {
* SUSPEND OCCURS HERE *
Fine but then the packet will arrive and we will wake back up process the
packet and wake the task. See suspend is just like a deep sleep. If we
went to sleep there and the packet arrival doesn't rewake the box the
driver was buggy.
reaches the end of its timeslice. At this point the application has not
had an opportunity to indicate in any way whether or not the packet has
altered its constraints in any way. What stops us from immediately
suspending again?
If my app level constraint before the packet is 'don't suspend' then my
constraint on receipt is 'don't suspend' so I won't suspend. If my
constraint is then lowered and I suspend I will suspend *after* the
lowering.
If my constraint is tightened then the decision to tighten is run under
the previous constraint. Which is fine, because if I have a case where I
must tighten my constraint within the tight constraint time I've screwed
up my app and need to fix it.
In reality almost all your userspace is going to look like 'trusted app'
or 'untrusted app' in droidthink and won't be transitioning in user space
(but may well be adding/losing kernel constraints)
This is good because it's another thing app authors don't have to care
about. It's good because apps can be run trusted/untrusted without
recompiling.
Alan
--
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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 18:40:19 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 07:34:40PM +0200, Peter Zijlstra wrote: we still need to be able to enter suspend while the system isn't idle. _WHY_!? Because if I'm running a kernel build in a tmpfs and I hit the sleep key, I need to go to sleep. Blocking processes on driver access isn't sufficient. Suspend as an idle state Suspend as a 'hand of God' choice. One is a performance optimisation the other is an operator executive decision. I'd prefer we avoided mixing them up. Everyone seems fairly happy with the current operator ordered suspend behaviour I believe ? -- 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 0/8] Suspend block api (version 8)
So PCs with current ACPI don't get opportunistic suspend capability. It probably won't be supported on the Commodore Amiga either - your point ? Actually, the reverse - there's no terribly good way to make PCs work with scheduler-based suspend, but there's no reason why they wouldn't work with the current opportunistic suspend implementation. If one works so does the other. In some cases, not all. It may be a latency constraint (in which case pm_qos is an appropriate mechanism), but instead it may be something like A key was pressed but never read or A network packet was received but not delivered. These don't fit into the pm_qos model, but it's state that you have to track. I never mentioned pm_qos, just latency *and* knowing what suspend states are acceptable. You need both. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 18:57:19 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 07:52:59PM +0200, Peter Zijlstra wrote: Shall we henceforth stop confusing forced suspend for laptops and powersaving a running system? If you want to support forced suspend for laptops and you want to avoid the risk of losing wakeups then you need in-kernel suspend blockers. It isn't suspend blockers or nothing. Look at the bigger picture. -- 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 0/8] Suspend block api (version 8)
No, it's not. Forced suspend may be in response to hitting a key, but it You are the only person here talking about 'forced' suspends. The rest of us are talking about idling down and ensuring we are always in a state we un-idle correctly. may also be in response to a 30 minute timeout expiring. If I get a WoL packet in the 0.5 of a second between userspace deciding to suspend and actually doing so, the system shouldn't suspend. I don't think that argument holds water in the form you have it What about 5 nanoseconds before you suspend. Now you can't do that (laws of physics and stuff). So your position would seem to be we have a race but can debate how big is permissible The usual model is At no point should we be in a position when entering a suspend style deep sleep where we neither abort the suspend, nor commit to a suspend/resume sequence if the events we care about occur and that is why the hardware model is Set wake flags Check if idle If idle Suspend else Clear wake flags Unwind and the wake flags guarantee that an event at any point after the wake flags are set until they are cleared will cause a suspend to be resumed, possibly immediately after the suspend. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 19:17:58 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 08:06:38PM +0200, Peter Zijlstra wrote: On Thu, 2010-05-27 at 18:59 +0100, Matthew Garrett wrote: On Thu, May 27, 2010 at 07:56:21PM +0200, Peter Zijlstra wrote: On Thu, 2010-05-27 at 18:52 +0100, Matthew Garrett wrote: If that's what you're aiming for then you don't need to block applications on hardware access because they should all already have idled themselves. Correct, a well behaved app would have. I thought we all agreed that well behaved apps weren't the problem? Ok. So the existing badly-behaved application ignores your request and then gets blocked. And now it no longer responds to wakeup events. It will, when it gets unblocked from whatever thing it got stuck on. It's blocked on the screen being turned off. It's supposed to be reading a network packet. How does it ever get to reading the network packet? Thats a stupid argument. If you write broken code then it doesn't work. You know if I do ls unopenedfifo it blocks too. There is a difference between dealing with apps that overconsume resources and arbitarily broken code (which your suspend blocker case doesn't fix either but makes worse). Can we stick to sane stuff ? -- 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 0/8] Suspend block api (version 8)
If one works so does the other. Not at all. The entire point of opportunistic suspend is that I don't care is currently in TASK_RUNNABLE or has a timer that's due to expire in 100msec - based on policy (through not having any held suspend blockers), I'll go to sleep. That's easily possible on PCs. Yes I appreciate what suspend blockers are trying to do. Now how does that connect with my first sentence ? -- 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 0/8] Suspend block api (version 8)
Sigh. No. Forced suspend is a fact of life
'Fact of life' isn't a useful reasoning basis. It tells me nothing about
how you got to that pronouncement.
Drivers should enable wakeups before they disable interrupts. So either
the packet hits the IRQ handler and the driver takes a suspend block
(aborting suspend in the process) or it doesn't, the GPE goes high and
the system resumes immediately after entering S3.
Set wake flags
Check if idle
If idle
Suspend
else
Clear wake flags
Unwind
and the wake flags guarantee that an event at any point after the wake
flags are set until they are cleared will cause a suspend to be resumed,
possibly immediately after the suspend.
Exactly. So the 5 nanosecond case is already handled. The interesting
case is where userspace makes the decision to go to sleep and starts
performing various housekeeping tasks before triggering the suspend. You
need some way to abort that suspend.
See above. You can't always abort the suspend you may have to go full
circle. Not only that but the exactly algorithm above can be applied
user to kernel as well as kernel to hardware.
Given your average app author I think I'd rather just run their suspend
then their resume handling back to back anyway because it's a single
tested code path, while half way through cases in apps will *never* get
tested or work properly.
So I guess if you are driving this rom user space (which I take it you
are given you talk about housekeeping)
foreach app we need to suspend
kick app to suspend (signal)
(policy) kick harder if needed (SIGSTOP/bitch/shall I
kill it dialogue)
rendezvous (apps now all sleeping or dead)
X
if (we still want to suspend) {
prod kernel
kernel suspends
kernel resumes
}
resume all the apps
The important msising bit from that is 'set wake flags'.
Now what actually happens here beyond point X. Any event that gets kernel
processed moves the task into RUNNING. Any event that occurs after that
point causes the hardware to go suspend/resume. If not the driver is
buggy.
Now replace kernel suspends with let the kernel know it can drop into
suspend level idle. The power management code will handle the races
beyond point X for you.
So for that you simply need a constraint to remain above suspend level
idle that you drop where it says prod kernel and pick up on resume
Alan
--
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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 20:29:26 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 08:25:38PM +0100, Alan Cox wrote: The scheduler can happily do this, the power management will also recognize STOPPED processes as no impediment to suspend. But wakeup events won't be delivered to STOPped processes, and there's Try the following cat pipe kill -STOP catpid echo wombats are cool pipe kill -CONT catpid it will echo wombats are cool The event was not lost. also the race of an application being in the middle of handling a wakeup event when you send it the signal. sigmask() -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 19:23:03 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 08:18:49PM +0200, Thomas Gleixner wrote: On Thu, 27 May 2010, Matthew Garrett wrote: Actually, the reverse - there's no terribly good way to make PCs work with scheduler-based suspend, but there's no reason why they wouldn't work with the current opportunistic suspend implementation. How does that solve the problems you mentioned above ? Wakeup guarantees, latencies ... Latency doesn't matter because we don't care when the next timer is due to expire. In your specific current implementation. It matters a hell of a lot in most cases. Alan -- 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 0/8] Suspend block api (version 8)
We were talking about PCs. Suspend-as-c-state is already implemented for OMAP. You were talking about PCs. Some of us are interested in the making Linux do the right thing not adding platform specific hacks all over the userspace of all the apps. So the only thing you are imposing to app writers is to use an interface which solves nothing and does not save you any power at all. It's already been demonstrated that the Android approach saves power. So do lots of other things Runnable tasks and QoS guarantees are the indicators whether you can go to opportunistic suspend or not. Everything else is just window dressing. As I keep saying, this is all much less interesting if you don't care about handling suboptimal applications. If you do care about them then I don't believe the Android one does either. It maybe handles a subset in a specific case. the Android approach works. Nobody has demonstrated a scheduler-based one that does. I would point you at the web, cgi scripts and the huge Linux server farms fielding billions of hits per second on crap cgi scripts. That doesn't mean the Android one is the right approach. Nobody has explained to me how you don't get synchronization effects in Android or indeed answered several of the questions pointing out holes in the Android model. The fact we are at rev 8 says something too - that the Android 'proof' isn't old or tested either ! Alan -- 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 0/8] Suspend block api (version 8)
The event was not lost. Not lost, but not delivered. So you need your policy agent to send it will be delivered next time the process wakes. It's not lost. SIGCONT when you receive any wakeup event, which either means proxying all your network traffic through your policy agent or having some mechanism for alerting the policy agent whenever you leave the deep idle state. You didn't mention that requirement last time. also the race of an application being in the middle of handling a wakeup event when you send it the signal. sigmask() Doesn't help - I may be hit by the signal between the poll() unblocking and me having the opportunity to call sigmask(). ppoll(). This is all existing solved stuff. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 18:59:20 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 07:56:21PM +0200, Peter Zijlstra wrote: On Thu, 2010-05-27 at 18:52 +0100, Matthew Garrett wrote: If that's what you're aiming for then you don't need to block applications on hardware access because they should all already have idled themselves. Correct, a well behaved app would have. I thought we all agreed that well behaved apps weren't the problem? Ok. So the existing badly-behaved application ignores your request and then gets blocked. And now it no longer responds to wakeup events. So you penalise well-behaved applications without providing any benefits to badly-behaved ones. I don't see how you put the first two sentences together and get the final one. When you beat up badly behaved apps that doesn't penalise well behaved ones. Forcing well behaved apps to make hundreds of extra calls to a complex blocker interface that also requires tons of kernel code and requires the application know platform policy and be recompiled if it changes - now that is punishing well behaved apps. A well behaved app should just work using standard existing APIs because that is how all the standard current well behaved apps are written [1]. Alan -- [1] I'm dying to see the suspend blocker patch for evolution ;) -- 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 0/8] Suspend block api (version 8)
That's the point -- suspend does not evaluate the system state
correctly because it doesn't have the necessary information. Suspend
The power management layer knows if the machine is idle, it has insight
into when the next wakeups will occur.
blockers are a way of providing it that information. They don't paper
over the problem; they solve it.
I'm still unconvinced (see Thomas examples as a starter) and even if they
did, the cure would be far worse than the disease.
The hardware world also shows how you handle suspend/resume event races
cleanly and in turn how we can handle right from user to hardware.
If you are dealing with a well behaved UI app then its event loop will
just do
if (message == UI_OFF) {
stop_background_stuff();
event_mask = stuff;
send_reply(NIGHT_NIGHT);
continue;
/* Back to poll() */
}
and power management does the rest. If an event wakes up the process
before we get to suspend in kernel we will wake it back up. If the event
gets in after pm decides to idle via suspend then it'll bounce through
the kernel back to a kernel resume, or potentially through a full
hardware/software suspend/resume bounce. You have one person who needs to
track the sequence - that's however is the guy sending UI_ON/UI_OFF
information and making the screen light up.
If you are dealing with a badly behaved app then you can't win because it
may not have the magic 'suspend blockers' or will do it wrong. You
can constrain it (brute force SIGSTOP after it refuses to UI_OFF for a
bit, or via nice or other QoS policy) but you can't assume it'll
magically managge itself. It is badly behaved so by that very
definition you can't assume it has correct magic goo in it. (Side note:
the extra magic goo and complexity of such blockers makes it more likely
the app will be buggy and thus 'badly behaved' !)
You can pick up which tasks are stopping the suspend fairly easily at the
moment via the ktrace interfaces. Maybe there is a case for making that
tidier but powertop and the like already demonstrate you can track this.
A cpu policy can obviously expose offenders itself directly to user
space. That (and indeed people not responding to UI_OFF) then lets your
phone do
boop
X is misbehaving
[Close it] [Allow this time] [Disable]
and also to provide stats for each wakeup a policy manager can spot
offenders (even those who are perhaps hard to see because they wake up a
little bit every so often but just enough to mess up pm). In the x86
world Arjan found a *lot* of that sort of stuff.
Your policy manager can also act on it. The current policy manager is
to fix app bugs and shows people up at conferences. I would imagine a
google policy would be to collate misbehaving app reports at google.com
and use them both to assist in 'educating' authors and in scoring app
store stuff.
As it is boop X is misbehaving is a powerful economic lever in a
pretty dynamic and functioning software market.
Google seem to be pretty good at statistical data, I can't believe
battery advice for apps on the app stores and indeed a 'where did my
battery go' pie chart are beyond their wit, or beyond the end user to
figure out.
There are a considerable number of people shipping power constrained
Linux devices without suddenely desperately needing extra magic potions.
Are they really all just that much smarter than Google ?
Alan
--
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 0/8] Suspend block api (version 8)
Well, what about when I want the machine to suspend _regardless_ of whether or not it's idle at the moment? That actually happens quite often to me. :-) I don't think it helps to combine them for this discussion ? I don't see much difference between that and ACPI S3 other than the memory contents are preserved in S3. It also is complete power off state - except for memory refresh and wakeup sources (which also may be active in S4). Agreed although I am pushed to think of many real world cases where it might matter. VM's maybe? Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 13:29:18 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: On Thu, 27 May 2010, Peter Zijlstra wrote: On Thu, 2010-05-27 at 13:04 -0400, Alan Stern wrote: Does this mean you believe echo mem /sys/power/state is bad and should be removed? Or echo disk /sys/power/state? They pay no attention to latencies or other requirements. Those are a whole different beast, those are basically a quick-off button like thing. Forced suspend is conceptually a very different beast from power-saving a running system. They may be different conceptually. Nevertheless, Android uses forced suspend as a form of power saving. Until better mechanisms are in place, it makes sense. For them, not for Linux. Several vendors have exciting kernel drivers that do things like binary patch other modules. Until better mechanisms are in place it does *NOT* make sense to merge such stuff. I don't care what they do in their own tree (consenting adults in their own home and all that) but what they do in the public tree is another matter. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 18:25:10 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 07:20:56PM +0200, Peter Zijlstra wrote: Suppose X (or whatever windowing system) will block all clients that try to draw when you switch off your screen. How would we not wake them when we do turn the screen back on and start servicing the pending requests again? How (and why) does the WoL (which may be *any* packet, not just a magic one) turn the screen back on? Well on my laptop today it works like this A WoL packet arrives The CPU resumes Depp process, chipset and laptop BIOS magic happens The kernel gets called The kernel lets interested people know a resume occurred The X server sees this X reconfigures the display X redraws the display (either by sending everyone expose events or by keeping the bits, not sure how it works this week as it has changed over time) My desktop re-appears Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 22:36:35 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 10:37:51PM +0100, Alan Cox wrote: On Thu, 27 May 2010 18:25:10 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: How (and why) does the WoL (which may be *any* packet, not just a magic one) turn the screen back on? Well on my laptop today it works like this A WoL packet arrives The CPU resumes Depp process, chipset and laptop BIOS magic happens The kernel gets called The kernel lets interested people know a resume occurred No it doesn't. The kernel continues executing anything that was on the Would you like to come and watch my laptop resume ? With printk's if you want. You appear at this point to be arguing that bumble bees can't fly, while watching one. runqueue before the scheduler stopped. If you're using idle-based suspend then there's nothing on the runqueue - the application that should be scheduled because of the event is blocked on writing to the screen. IFF its your bogus example IFF you don't have any task waiting for resume notifications (ie its not X) So take the PC desktop case and for simplicity sake lets assume the X application in question has either filled the socket (unlikely) or is mid query request so blocked on the socket. The important line then is 'The kernel lets interested people know a resume occurred' Interesting people includes X X therefore ends up on the runqueue X gets the display back in order X completes processing the outstanding X request and replies The application continues If I was blocked on say serial output then the resume is going to wake the serial driver, which will transmit the queue, which will wake the app. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 17:38:03 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: and power management does the rest. If an event wakes up the process before we get to suspend in kernel we will wake it back up. Okay, and the kernel never suspends. We _are_ talking about a kind of forced suspend, right? No ? We are talking about just letting power management solve the whole problem for us. If the event gets in after pm decides to idle via suspend then it'll bounce through the kernel back to a kernel resume, No, it won't. That's the problem suspend blockers were meant to solve. The event winds up sitting in a kernel queue, the PM core doesn't know about it (that's what I meant above -- the PM core doesn't know as much as you seem to think it does), and the suspend takes place regardless. No - because we are not forcing the suspend. The app must go idle. If you force the suspend of running processes then yes the entire thing goes castors up - which is why it's a bad idea to do so. or potentially through a full hardware/software suspend/resume bounce. It would be okay if that happened. But once the event gets into the kernel and the hardware IRQ source has turned off, there's nothing to cause the resume. Read the discussion about how the race is avoided at the hardware level. That race is I think not there and furthermore most drivers get it right already. There are several cases 1. IRQ during app layer (ie policy in user space) asking applications to go passive - The event occurs, we undo the app layer policy, easy (or app wakes process and we let it fall through) 2. IRQ after the app layer quiesces its clients - The task wakes, the app layer won't see it - the app layer allows suspend as an idle mode. Not a problem - the app is running the cpu policy manager will see this and not suspend until the app has been asleep for a bit. The app may well of course tell the UI layer 'hey I want you back on' and it take you back to the full on case. 3. IRQ after kernel suspend begins - The driver will refuse the suspend, we don't suspend, we unwind the resume so far, the app wakes, we propogate stuff back up to user space whose policy manager unwinds its position 4. IRQ after driver has done its final checks - Wake up lines are set - We suspend - We immediately get resumed - We follow the full resume path This is I believe robust (and has been implemented on some non x86 boxes). It depends on not forcing running tasks into suspend. That is the key. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 23:55:13 +0200 Rafael J. Wysocki r...@sisk.pl wrote: On Thursday 27 May 2010, Alan Cox wrote: If one works so does the other. Not at all. The entire point of opportunistic suspend is that I don't care is currently in TASK_RUNNABLE or has a timer that's due to expire in 100msec - based on policy (through not having any held suspend blockers), I'll go to sleep. That's easily possible on PCs. Yes I appreciate what suspend blockers are trying to do. Now how does that connect with my first sentence ? I guess what Matthew wanted to say was that you couldn't use ACPI S3 as a very deep CPU idle state, because of the way wakeup sources are set up for it, while you could use it for aggressive power management with suspend blockers as proposed by Arve. Which is a nonsense. Because the entire Gnome desktop and KDE, and OpenOffice and Firefox and friends would need fitting out with suspend blockers. x86 hardware is moving to fix these problems (at least on handheld devices initially). Look up the C6 power idle, and S0i1 and S0i3 standby states. I reckon the laptop folks can probably get the hardware fixed well before anyone can convert the entire PC desktop to include blockers. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 23:09:49 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 11:08:06PM +0100, Alan Cox wrote: This is I believe robust (and has been implemented on some non x86 boxes). It depends on not forcing running tasks into suspend. That is the key. We've already established that ACPI systems require us to force running tasks into suspend. How do we avoid the race in that situation? Android phones do not have ACPI. Embedded platforms do not have ACPI. MID x86 devices do not have ACPI. I would imagine the existing laptops will handle power management limited by the functionality they have available. Just like any other piece of hardware. Alan -- 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 0/8] Suspend block api (version 8)
The kernel performs no explicit notification to userspace. With legacy For a PC ACPI using type device /proc/acpi/events which wakes acpid which wakes gnome-power-manager which wakes half the universe Do we need a better more generic version of the events files - maybe but thats a rather different kettle of fish to suspend blockers. Alan -- 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 0/8] Suspend block api (version 8)
On Thu, 27 May 2010 23:36:05 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 11:23:57PM +0100, Alan Cox wrote: On Thu, 27 May 2010 23:09:49 +0100 Matthew Garrett mj...@srcf.ucam.org wrote: On Thu, May 27, 2010 at 11:08:06PM +0100, Alan Cox wrote: This is I believe robust (and has been implemented on some non x86 boxes). It depends on not forcing running tasks into suspend. That is the key. We've already established that ACPI systems require us to force running tasks into suspend. How do we avoid the race in that situation? Android phones do not have ACPI. Embedded platforms do not have ACPI. MID x86 devices do not have ACPI. It doesn't matter. Right now there's a race condition in terms of wakeup events on ACPI systems. What's your proposal for fixing that? I see it as a different problem - and one that seems to be minimally pressing to most users jduging by the amount of noise it hasn't caused in the past seven odd years. This started because the Android people came to a meeting that was put together of various folks to try and sort of the big blockage in getting Android and Linux kernels back towards merging. I am interested right now in finding a general solution to the Android case and the fact it looks very similar to the VM, hard RT, gamer and other related problems although we seem to have diverged from that logic. I dont think it particularly useful to go off on a mostly unrelated wild goose chase into ACPI land, especially one based on a premise of changing all the apps when the hardware will end up fixed faster. Alan -- 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 0/8] Suspend block api (version 8)
The approach with user space power manager suggested by Dmitry and Alan Stern may work, but it still assumes some kind of suspend blockers to be present in the kernel. If we reject that too, I wonder what approach Google is supposed to use and still get the same battery life they get with suspend blockers. I'm getting less convinced it needs suspend blockers at all for this case, assuming that you are willing to have a policy that is based on - assuming apps play nicely - having the information to user space you need (who woke us, who blocked us, events) - dealing with offenders primarily from user space using that information I'm fairly happy about the following so far - we should have a common interface for seeing some pm events (like duh ?) but it does need careful thought so the watcher doesn't change the behaviour and break it. (Message We are suspending, gosh someone is running to receive the message, resume being the obvious case) - Suspend is (for many platforms) just a cotinuation down the power chain. Demonstrated and implemented on ARM. Very much the direction of S0i1/S0i3 on x86 MID devices. Proved by the fact it has been done and made to work, and by reading the Moorestown PR. - Given a non forced (that is 'idle down') transition to a suspend level we can implement a 'suspend as idle' on many embedded platforms in a manner which is not racy at kernel level. Apparently implemented already on ARM - Given a non forced transition to such a suspend level and the reporting of certain events we can do a full user space managed graphical UI type environment policy in a race free fashion - With notification of who caused a resume and maybe a bit of other general stat gathering it is possible to identify and handle abuses of power resource. Proved by the fact we can do this with powertop but more elegance in the interfaces would be nice. I am not sure if a pm event is what is needed for this or a sum 'hardware triggered wake up' event. I accept that current ACPI based laptops probably couldn't make use of such a feature but I don't think this is important at the moment. A resource constraint model might help further in the ACPI case. It's useful for other stuff but it might well be a distraction and implementation detail in terms of the basic question about what is needed for something like Android. At this point the input of the Android team and the Nokia people would be rather more useful to me. Alan -- 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] Suspend block api (version 8)
I don't think x86 is relevant anyway, it doesn't suspend/resume anywhere near fast enough for this to be usable. Yet... My laptop still takes several seconds to suspend (Lenovo T500), and resume (aside from some userspace bustage) takes the same amount of time. That is quick enough for manual suspend, but I'd hate it to try and auto-suspend. This is an area where machines are improving and where the ability to do stuff like autosuspend, the technology like the OLPC screen and so on create an incentive for the BIOS and platform people to improve their bits of it. So yes, I do think merging this will delay the effort in fixing userspace, simply because all the mobile/embedded folks don't care about it anymore. The mobile space probably doesn't care too much about many of the large bloated desktop apps anyway and traditional embedded generally has a very small fixed application set where the optimise both halves of the system together. Alan -- 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 0/8] Suspend block api (version 8)
Really, what are you getting at? Do you deny that there are programs, that prevent a device from sleeping? (Just think of the bouncing cows app) And if you have two kernels, one with which your device is dead after 1 hour and one with which your device is dead after 10 hours. Which would you prefer? I mean really... this is ridiculous. The problem you have is that this is policy. If I have the device wired to a big screen and I want cows bouncing on it I'll be most upset if instead it suspends. What you are essentially arguing for is for the kernel to disobey the userspace. It's as ridiculous (albeit usually less damaging) as a file system saying Ooh thats a rude file name, the app can't have meant it, I'll put your document soemwhere else The whole API feels wrong to me. It's breaking rule #1 of technology You cannot solve a social problem with technology. In this case you have a social/economic problem which is crap code. You solve it with an economics solution - creative incentives not to produce crap code like boxes that keep popping up saying App XYZ is using all your battery and red-amber-green powermeter scores in app stores. That said if you want technical mitigation I think it makes more sense if you look at it from a different starting point. The starting point being this: We have idling logic in the kernel and improving this helps everyone. What is needed to improve the existing logic ? - You don't know which processes should be ignored for the purpose of suspend (except for kernel threads) and there is no way to set this - You don't know whether a move from a deep idle to a 'suspend' (which is just a really deep idle in truth anyway) might break wakeups requirements because a device has wake dependencies due to hardware design (eg a port that has no electronics to kick the box out of suspend into running). This is a problem we have already. [1] That maps onto two existing ideas Sandboxing/Resource Limits: handling apps that can't be trusted. So the phone runs the appstore code via something like setpidle(getpid(), something); exec() where 'something' is a value with meaning to both user space and to the existing idling logic in the kernel that basically says to what extent it is permitted to block idling/suspend. That also seems to tie into some of the realtime + idle problems. This I think deals with Kevin Hillman's thoughts on dealing with untrustworthy app code more cleanly and avoids the need for userspace hackery like the blocker API. And an entirely in kernel API where device drivers can indicate that in their current situation they require that the power level doesn't drop below some limit unless user requested. This is really important because the platform vendor of the phone/pda/tablet whatever effectively owns the kernel - so it's *their* problem, *their* control, *their* hardware and they can make it work as best for the device. Best of all it means its all free software stuff so if the vendor screws up you can still fix your phone. Implementation-wise it probably ties into setpidle, its simply that a task has a pair of idle values, a dynamic one and a base one, the dynamic one being the base one but updatable temporarily by drivers. Alan -- [1] Note I disagree with Kevin here on static/dynamic power management. There are IMHO two types of PM but they are 'user invoked' and 'automatic'. Static simply means it's not been made fast enough yet but its just a policy divide dependant on the users 'acceptable' resume time (which for hard RT may just as well rule out some more usual power states) -- 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 0/8] Suspend block api (version 8)
Nonetheless, I really think the kernel needs to allow for the android way of power saving. It misses out on a big feature and a big user-base if not. That seems to me to be conflating models of behaviour and implementations. This is a _big_ plus for attracting 3rd party programs. (And of course the thing you don't like). You would do better to concentrate on technical issues that the assignment of malicious intent to other parties. Alan -- 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 0/8] Suspend block api (version 8)
I'm not saying that your argument is not valid. But why don't you look at suspend blockers as a contract between userspace and kernelspace? An Opt-In to the current guarantees the kernel provides in the non-suspend case. It is a contract - but not the right one. You are removing autonomy from the kernel when only the kernel can measure the full picture and when the kernel is actually supposed to be responsible for resource management. On the other hand, applications can say, they don't need that much power and userspace guaranties and not take a suspend blocker. Even the the model is wrong. I don't think opportunistic suspend is a policy decision by the kernel. it is something new. Something which currently only the android Disagree. It's an arbitary and misleading divide that happens to reflect a specific vendors current phones. Worse yet it may not reflect their own future products. It assumes for example that their is some power level that is 'suspend' that is singular, application understood and can be effectively user space managed. Big assumptions and not ones that seem to be sensible. It also breaks another rule - when the hardware changes your application blocker policies will be wrong. Do you want multiple hand optimised copies of each app ? Take a look at what happened to CPU designs where the assumption was you'd recompile the app for each CPU version to get any useful performance. If you are instead expressing it as must be able to respond in time X and must be able to wake up from an event on an active device then your interface is generic and hardware independant. If bouncing cows says 'need to wake up every 0.3 seconds you want the kernel to decide how best to do that. It will vary by hardware. On todays desktop PC thats probably a low power state limit. On some current embedded hardware it might be a special deep sleep mode. On one or two devices it might be 'suspend'. It might also be that the properties have been set to 2 seconds already so it gets told it can't have 0.3. The app cannot be allowed to know platform specific stuff or your portability comes apart and you end up with a disaster area where each app only comes on a subset of devices. Express the requirement right and you get a simple clean interface that continues to work. Express it wrong and you get a mess. userspace implements / supports. If you don't want to suspend while looking at the bouncing-cow, you have to take a suspend blocker and make yourself a user-visible power-eater, or don't do Thats a very big hammer and it doesn't express what I actually want, which is to allow the cows to run as efficiently as possible. echo opportunistic /sys/power/policy in the first place. But you can do this properly by having a per process idle requirement, and that can encompass things like hard real time as well (or even gaming). The suspend blockers break all the global policy, don't solve real time and don't allow for sensible expansion models. They don't solve our existing wakeup versus device problems either. How does this address the loss of wakeup events while using suspend? (For example the 2 issues formulated by Alan Stern in [1]) In this environment the problem cannot occur in the first place unless there are kernel code bugs, if there are then they are in GPL code and can be fixed. But you are mixing up interfaces and implementations which I find is usually a bad idea. Doing the right thing badly gives you an interface to an implementation you can later fix. Doing the wrong thing well leaves you stuck down a hole. p.s.: d...@schatten /usr/src/linux $ grep -r setpidle . Yes I know - no point having a new function which has an in use name is there ? It's trivial to add per process idling or wakeup requirements. Alan -- 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 0/8] Suspend block api (version 8)
The power efficiency of a mobile device is depending on a sane overall software stack and not on the ability to mitigate crappy software in some obscure way which is prone to malfunction and disappoint users. Even if you believe the kernel should be containing junk the model that works and is used for everything else is resource management. Not giving various tasks the ability to override rules, otherwise you end up needing suspend blocker blockers next week. A model based on the idea that a task can set its desired wakeup behaviour *subject to hard limits* (ie soft/hard process wakeup) works both for the sane system where its elegantly managing hard RT, and for the crud where you sandbox it to stop it making a nasty mess. Do we even need a syscall or will adding RLIMIT_WAKEUP or similar do the trick ? Alan -- 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 0/8] Suspend block api (version 8)
suspend blockers are completely backwards as they basically disable the kernels ability to do resource management. I don't think this is a defect in the approach but the point of it. I think it's both. It's the point of it, and that itself is a defect. Its designed wrongly. drivers code is needed anyway. What looses the kernel by implementing suspend blockers, and later a more finegrained approach and mapping the userspace part of suspend blockers on that finegrained approach later on? The Linux approach is to do the job right. That means getting the interface right and so it works across all the interested parties (or as close as we can get). - how much delay of wakeups is tolerated (missing interface) Doesn't solve the segregation problem and is probably overkill for most applications. I see this as an orthogonal thing (i.e. not affecting this merge). The key question that matters for suspend management is 'what wakeup delay is tolerated'. So it's absolutely fundamental. True. But I wouldnt say, that it is the linux kernel who should force this politics onto its users. This is the Linux way of doing things. It's like the GPL and being shouted at by Linus. They are things you accept when you choose to take part. Google chose to use Linux, if they want a feature upstream then the way you get it there is to figure out how to solve the real problem and make *everyone* (within reason) happy. We now have suggestions how to do the job properly so the right thing is probably to go and explore those suggestions not merge crap. Merging crap won't help anyway. The rest of the kernel community can still simply stonewall those interfaces, and a volunteer community has ways of dealing with abuse of process, notably by simply not getting around to, or implementing things directly contrary to the crap bits. So it's not even in the interest of people to play political games. Even if you get away with in the short term the people who rely on the junk will end up out on a limb and holding the baby when the crap hits the fan (see reiserfs) Alan -- 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 0/8] Suspend block api (version 8)
On Wed, 26 May 2010 15:30:58 -0700 Arve Hjønnevåg a...@android.com wrote: On Wed, May 26, 2010 at 6:16 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: Really, what are you getting at? Do you deny that there are programs, that prevent a device from sleeping? (Just think of the bouncing cows app) And if you have two kernels, one with which your device is dead after 1 hour and one with which your device is dead after 10 hours. Which would you prefer? I mean really... this is ridiculous. The problem you have is that this is policy. If I have the device wired to a big screen and I want cows bouncing on it I'll be most upset if instead it suspends. We never suspend when the screen is on. If the screen is off, I would not be upset if it suspends. This is policy and platform specific. The OLPC can suspend with the screen on. Should I write my app to know about this once for Android and once for OLPC (and no doubt once for Apple). In the OLPC case cows could indeed suspend to RAM between frames if the wakeup time was suitable. My app simply should not have to know this sort of crap, that's the whole point of an OS. What you are essentially arguing for is for the kernel to disobey the userspace. No I'm not. User-space asked the kernel to suspend when possible. Suspend is an existing kernel feature. All opportunistic suspend adds is the ability to use it safely when there are wakeup event you cannot afford to ignore. Don't get me wrong - opportunistic suspend isn't the problem. Suspend blockers are - or more precisely - the manner in which they express intent from userspace. Opportunistic suspend is wonderful stuff for all sorts of things and if it persuades people like netbook manufacturers to think harder, and Linux driver authors to optimise their suspend/resume paths we all win. Our actual stating point is this: Some systems can only enter their lowest power state from suspend. So we added an api that allowed us to use suspend without loosing wakeup events. Since suspending also improves our battery life on platforms that enter the same power state from idle and suspend and we already have a way to safely use suspend, we would be crazy not to use it. Opportunistic suspend isn't special. Suspend is just a very deep idle. In fact some of the low power states on processors look little different to suspend - the OS executes a whole pile of CPU state saving and cache flushing. It might be a hardware state machine, it might be buried in firmware or it might be quite explicit (eg mrst). So we already have differing degrees of doing additional work in different states. User triggered suspend is a bit special in that the user is usually right in that case to override the power management policy. Note I'm not suggesting we run off and restructure all our power management code to take this view right now. I'm suggesting we need a clean 'opportunistic suspend is not special' view by user space. How the kernel handles this is addressible later without app breakage, but only if we get the interface wrong to begin with. Sandboxing/Resource Limits: handling apps that can't be trusted. So the phone runs the appstore code via something like Sandboxing is problematic on Android since there are a lot of cross process dependencies. When a call comes in I don't know where the name and picture to display comes from. With suspend blockers we block suspend when we get notified that we have an incoming call and we can call into any process and get a response. But you can express user suspend blocking in this manner. Your call incoming code would say 'I want good latency'. As someone needs good latency the box won't suspend. If your approach is to start with an initial 'anyone can set a low latency we don't care' then anyone can block suspends. Equally your call handling example is about latency not about suspend. You want the phone to stay on, to fetch a picture and display it promptly. So what are expressing 'I am using device 'screen' please keep it live' (which may or may not block suspend - see OLPC). I guess your display server and kernel support manage this bit. 'I want the photo to appear in a resonable timescale' (latency). It's not a suspend question - an imaginary non suspend idle mode with a 20 second latency would be just as annoying yes ? At the moment we have a very real bigger problem that your problem is part of. - Hard real time people want to be able to limit the CPU sleeping behaviour based upon what tasks are running - Certain gaming types want their boxes to be good power citizens except when committing digital mass murder. Right now that involves wrapping the game in a script with bits occurring as root and that generally breaks if the game crashes so the script doesn't run nicely on exit - Virtual machine people desperately want to see latency data to help schedule stuff in a power efficient manner. In a virtual machine environment
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
stable and software has to cope with fewer events. We've lived without it for quite some time, though. We lived without graphics for quite some time, without 64bit for quite some time ;) Alan -- 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] gpiolib: introduce set_debounce method
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index cd85fd1..ed1ed74 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1461,9 +1461,14 @@ int gpio_set_debounce(unsigned gpio, unsigned debounce)
spin_lock_irqsave(gpio_lock, flags);
+ chip = desc-chip;
+ if (!(chip-flags GPIO_FLAG_DEBOUNCE)) {
+ spin_unlock_irqrestore(gpio_lock, flags);
+ return 0;
+ }
+
If you add the feature check then presumably someone trying to do
debounce on a port without the feature isn't paying attention so this
should WARN at the very least. Also it shouldn't be a return 0.
This however seems a bit excessive and inconsistent. Every other function
simply returns -EINVAL if the request is unsupported. So not only does it
complicate the code it makes the code inconsistent with its existing
regular behaviour. The initial patch is consistent, regular and follows
expected gpiolib behaviour in all respects.
that could be used later for adding debounce emulation for chips that
doesn't support hw debouncing.
You don't need flags for this - the request will just start working if
someone adds the feature.
GPIO is almost always fairly tightly platform bound so features only
existing on certain ports is fine. The platform vendor will have made
sure they relevant ports have suitable debounce facilities.
Alan
--
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] gpiolib: introduce set_debounce method
One of the earlier examples of why to want debouncing (in the original discussion, not this one) was for buttons (one signal per button press) and the hardware designers don't necessarily hook buttons up to only GPIOs with hardware debounce. So code to kick in debounce there needed to be sensitive to whether or not the capability was available ... and moreover, whether the debounce scale was adequate to compensate the contact chatter. When not, then software debounce was inescapable. And someone who needs it can contribute it. The proposed patch causes no problems in this area. Alan -- 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] gpiolib: introduce set_debounce method
+EXPORT_SYMBOL_GPL(gpio_set_debounce); nitlet: I suspect this function is taking gpio_lock sooner than it strictly needs to. Find-tuning that would decrease contention by an insignificant amount ;) We need this for Intel MID boxes as well Andrew - so its a generic need. Alan -- 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] serial: Add OMAP high-speed UART driver.
+ 213 = /dev/ttyO0 OMAP serial port 0 + 214 = /dev/ttyO1 OMAP serial port 1 + 215 = /dev/ttyO2 OMAP serial port 2 + 216 = /dev/ttyO3 OMAP serial port 3 I think it might be best to allocate a dynamic tty major nowdays Other problem is that I don't see where you hold the port lock over all uses of port-tty. That is an assumption of the serial layer and done in the original 8250 driver correctly. Rest looks good and I agree that OMAP is different enough from 8250 that it makes sense not to further clutter the 8250 driver. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] [PATCH] watchdog_info separation and constify
On Tue, 19 Jan 2010 22:17:59 +0100 Wim Van Sebroeck w...@iguana.be wrote: Hi All, please comment on following patch. Why move them out - why not just make them const ? -- 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] serial: 8250: add UPF_NO_EMPTY_FIFO_READ flag
On Tue, 17 Nov 2009 15:16:55 -0600 Vikram Pandita vikram.pand...@ti.com wrote: OMAP3630 and OMAP4430 UART IP blocks have a restriction wrt RX FIFO. Empty RX fifo read causes an abort. OMAP1/2/3 do not have this restriction. Nothing wrong with the requirement but I think it would keep 8250.c a lot cleaner if you implemented that in the serial_in method for the OMAP3630/4430. It would also mean anyone adding a UART_RX or any cases you miss don't break in future This is especially true now as you can pass a private serial_in/serial_out method in the port register function. Care to submit an alternate patch doing it that way, or see any reason for not ? Alan -- 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
