libata-acpi: summary, problems, questions and proposal
Hello, ATA and ACPI crowd. This mail tries to summarize the current state of libata ACPI support and establish consensus how it should be improved. If you're familiar with ACPI or the current libata-acpi implementation, please try to answer questions in section 3. Table of Contents 1. Summary of the current libata ACPI support 2. Problems of the current implementation 3. Questions regarding ATA ACPI spec and the current implementation 4. Proposal for improvements 1. Summary of the current libata ACPI support Currently there are two branches of acpi support. One in git tree libata-dev (also merged into mainline) the other in -mm tree. The one in -mm tree contains additional _GTM/_STM support and pata_acpi.c implementation by Alan Cox. The additional part currently only in the -mm tree is necessary and needs to be merged but there is a disagreement regarding how pata_acpi should be done. ACPI support implementation in libata-dev supports both IDE and SATA ACPI object layouts and subset of ATA ACPI methods - _SDD and _GTF. It incorrectly uses ap-cbl (the port's cable type) to choose between the two ACPI layouts. Association between the host and its ACPI object is performed every time ACPI methods are invoked but the association between an ATA device and its ACPI object is cached in ata_device object. If ap-cbl indicates SATA cable type, _SDD is invoked every time the associated ATA device is configured (after reset and during revalidation). Also, _GTF and the resulting TFs are executed right after each _SDD. If ap-cbl does not indicate SATA cable type, ACPI support in libata-dev currently doesn't do anything because in some cases _GTF depends on _STM being called before it and executing _GTF before _STM causes errors in ACPI layer. As _GTM/_STM support in -mm is only used with pata_acpi yet, the situation is similar there too. Nothing is done if cable type isn't SATA. 2. Problems of the current implementation 2-1. Associating ATA host/device with their ACPI objects is broken. For one, it uses ap-cbl to determine which layout to use but this isn't dependent on cable type. This is dependent on the programming interface of the controller. If it implements or emulates SFF IDE interface, the IDE ACPI layout should be used. If the controller implements its own native SATA aware programming interface such as ahci or sil24, it should use SATA native layout. 2-2. Missing proper _GTM/_STM support. As stated above, although -mm contains _GTM/_STM support, it does not hook it to regular exception handling path and thus _GTF cannot be used in a lot of cases. 2-3. Misplaced _GTF hook. _GTF currently is called prior to every device configuration. This is unnecessary and incorrect. The ACPI spec specifies that _GTM/_STM and _GTF should be executed during suspend/resume cycles not on every reset or reconfiguration. This, for example, causes the following problem. _STM should be called before _GTF and, in turn, _GTF should be called before _STM, so, during initial detection, we should be doing _GTM - _STM - _GTF sequence. 3. Questions regarding ATA ACPI spec and the current implementation Please feel free to point out errors and it would be great if you can include or point to example .dat/.dsl files when answering. 3-1. In all ACPI specs form 1.0 to 3.0a, the example IDE hierarchy looks like the following. \_SB\PCI0\IDE1\{_ADR,_GTM,_STM,_PR0} \DRV1\{_ADR,_GTF} \DRV2\{_ADR,_GTF} \IDE2\{_ADR,_GTM,_STM,_PR0} \DRV1\{_ADR,_GTF} \DRV2\{_ADR,_GTF} Where the description of IDEx\_ADR is Indicates address of the channel on the PCI bus. But actual IDE ACPI hierarchy looks like the following. \_SB\PCI0\PCIDEVX\_ADR \IDE1\{_ADR,_GTM,_STM,_PR0} \DRV1\{_ADR,_GTF} \DRV2\{_ADR,_GTF} \IDE2\{_ADR,_GTM,_STM,_PR0} \DRV1\{_ADR,_GTF} \DRV2\{_ADR,_GTF} Where PCIDEVX\_ADR indicates the address of PCI device on the PCI bus while IDEx\_ADR indicates the channel number on the PCI device not the PCI bus. Is this a mistake in the spec or am I misreading it? 3-2. sata_get_dev_handle() looks for the ACPI object associated with the PCI host of the given ATA host by matching the PCI device's _ADR in the containing PCI bus, but pata_get_dev_handle() gets the ACPI device handle for the PCI device by simply invoking DEVICE_ACPI_HANDLE() on the generic device for the host device and verifies the ACPI object for the containing bus has the matching bus number in its _ADR. a. Why can't we just use DEVICE_ACPI_HANDLE() on host-dev in SATA case? What's the difference between IDE and SATA ACPI objects? AFAICS, they are the same PCI device
[git patches] libata fixes
This disables libata ACPI, among other things. Please pull from 'upstream-linus' branch of master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git upstream-linus to receive the following updates: drivers/ata/ahci.c | 21 +++- drivers/ata/libata-acpi.c |8 ++-- drivers/ata/libata-core.c |6 ++- drivers/ata/libata-eh.c | 66 -- drivers/ata/libata.h|2 +- drivers/ata/pata_pdc202xx_old.c |2 +- 6 files changed, 71 insertions(+), 34 deletions(-) Alan Cox (1): pata_pdc202xx_old: LBA48 bug Conke Hu (1): ahci.c: walkaround for SB600 SATA internal error issue Jeff Garzik (1): [libata] Disable ACPI by default; fix namespace problems Paul Rolland (1): ata: NCQ is broken on Maxtor 6L250S0 Tejun Heo (1): libata: IDENTIFY backwards for drive side cable detection diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index dc7b562..fd27227 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -80,6 +80,7 @@ enum { board_ahci_pi = 1, board_ahci_vt8251 = 2, board_ahci_ign_iferr= 3, + board_ahci_sb600= 4, /* global controller registers */ HOST_CAP= 0x00, /* host capabilities */ @@ -168,6 +169,7 @@ enum { AHCI_FLAG_NO_NCQ= (1 24), AHCI_FLAG_IGN_IRQ_IF_ERR= (1 25), /* ignore IRQ_IF_ERR */ AHCI_FLAG_HONOR_PI = (1 26), /* honor PORTS_IMPL */ + AHCI_FLAG_IGN_SERR_INTERNAL = (1 27), /* ignore SERR_INTERNAL */ }; struct ahci_cmd_hdr { @@ -362,6 +364,18 @@ static const struct ata_port_info ahci_port_info[] = { .udma_mask = 0x7f, /* udma0-6 ; FIXME */ .port_ops = ahci_ops, }, + /* board_ahci_sb600 */ + { + .sht= ahci_sht, + .flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | + ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | + ATA_FLAG_SKIP_D2H_BSY | + AHCI_FLAG_IGN_SERR_INTERNAL, + .pio_mask = 0x1f, /* pio0-4 */ + .udma_mask = 0x7f, /* udma0-6 ; FIXME */ + .port_ops = ahci_ops, + }, + }; static const struct pci_device_id ahci_pci_tbl[] = { @@ -399,7 +413,7 @@ static const struct pci_device_id ahci_pci_tbl[] = { PCI_CLASS_STORAGE_SATA_AHCI, 0xff, board_ahci_ign_iferr }, /* ATI */ - { PCI_VDEVICE(ATI, 0x4380), board_ahci }, /* ATI SB600 non-raid */ + { PCI_VDEVICE(ATI, 0x4380), board_ahci_sb600 }, /* ATI SB600 non-raid */ { PCI_VDEVICE(ATI, 0x4381), board_ahci }, /* ATI SB600 raid */ /* VIA */ @@ -1067,8 +1081,11 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat) if (ap-flags AHCI_FLAG_IGN_IRQ_IF_ERR) irq_stat = ~PORT_IRQ_IF_ERR; - if (irq_stat PORT_IRQ_TF_ERR) + if (irq_stat PORT_IRQ_TF_ERR) { err_mask |= AC_ERR_DEV; + if (ap-flags AHCI_FLAG_IGN_SERR_INTERNAL) + serror = ~SERR_INTERNAL; + } if (irq_stat (PORT_IRQ_HBUS_ERR | PORT_IRQ_HBUS_DATA_ERR)) { err_mask |= AC_ERR_HOST_BUS; diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index c428a56..03a0acf 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -305,7 +305,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, *gtf_address = 0UL; *obj_loc = 0UL; - if (noacpi) + if (libata_noacpi) return 0; if (ata_msg_probe(ap)) @@ -531,7 +531,7 @@ static int do_drive_set_taskfiles(struct ata_port *ap, ata_dev_printk(atadev, KERN_DEBUG, %s: ENTER: port#: %d\n, __FUNCTION__, ap-port_no); - if (noacpi || !(ap-cbl == ATA_CBL_SATA)) + if (libata_noacpi || !(ap-cbl == ATA_CBL_SATA)) return 0; if (!ata_dev_enabled(atadev) || (ap-flags ATA_FLAG_DISABLED)) @@ -574,7 +574,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap) unsigned long gtf_address; unsigned long obj_loc; - if (noacpi) + if (libata_noacpi) return 0; /* * TBD - implement PATA support. For now, @@ -636,7 +636,7 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix) struct acpi_object_list input; union acpi_object in_params[1]; - if (noacpi) + if (libata_noacpi) return 0; if (ata_msg_probe(ap)) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index bf327d4..f1f595f 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -93,8 +93,8 @@ static int ata_probe_timeout = ATA_TMOUT_INTERNAL / HZ;
Re: [git patches] libata fixes
Jeff Garzik wrote: This disables libata ACPI, among other things. If a -rc6 is possible, that would be quite nice... Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
On Tue, 2007-03-27 at 01:46 +0800, Jeff Chua wrote: On 3/27/07, Thomas Gleixner [EMAIL PROTECTED] wrote: It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can suspend and resume from RAM (s2ram). Even better, it works with/without CONFIG_NO_HZ. Does the patch below fix the HPET_TIMER=y case ? Thomas, I tried, but it didn't help. Upon resume from ram, date still didn't advance. Can you please issue a SysRq-Q in this situation and provide the dmesg output ? Thanks, tglx - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA patches
On Wed, 28 Mar 2007 01:08:52 +0100 Matthew Garrett [EMAIL PROTECTED] wrote: On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote: For reference this is what I am currently using with 2.6.21-rc4-mm1 and it is working for all my test cases so far: Its basically Kyle's patch with a libata switch to turn it on/off and some minor fixups from the original patch as posted Fails for me on a Macbook Pro with: Ok thanks. This is interesting as the only thing new it is doing is asking for the HPA size. Does I think explain the problem however: Can you move the HPA setting call to after the mode has been set and see if that makes the problem vanish ? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATA ACPI (was Re: Linux 2.6.21-rc5)
Hi! So if you have reported a regression in the 2.6.21-rc series, please check 2.6.21-rc5, and update your report as appropriate (whether fixed or still problems with xyzzy). [just got back from vacation, or would have sent this earlier] FWIW, I'm still leaning towards disabling libata ACPI support by default for 2.6.21. Upstream has Alan's fix for the worst PATA problems, but for different reasons, I think PATA ACPI and SATA ACPI support in libata does not feel quite ready for prime time in 2.6.21. Scream now, or hold your peace until 2.6.22... :) I second disabling ACPI for 2.6.21. Ugh.. does that mean we'll have 'regression reports' as in 'it worked ok in -rc5, broken in final? Well, suspend is currently so broken that we'll be flooded by reports, anyway, but could we get at least define in code so that we can tell users to flip it? Or maybe it is enough to make libata dependend on EXPERIMETAL? ...making it dependend on BROKEN should be definitely enough... -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATA ACPI (was Re: Linux 2.6.21-rc5)
Pavel Machek wrote: Hi! So if you have reported a regression in the 2.6.21-rc series, please check 2.6.21-rc5, and update your report as appropriate (whether fixed or still problems with xyzzy). [just got back from vacation, or would have sent this earlier] FWIW, I'm still leaning towards disabling libata ACPI support by default for 2.6.21. Upstream has Alan's fix for the worst PATA problems, but for different reasons, I think PATA ACPI and SATA ACPI support in libata does not feel quite ready for prime time in 2.6.21. Scream now, or hold your peace until 2.6.22... :) I second disabling ACPI for 2.6.21. Ugh.. does that mean we'll have 'regression reports' as in 'it worked ok in -rc5, broken in final? Well, suspend is currently so broken that we'll be flooded by reports, anyway, but could we get at least define in code so that we can tell users to flip it? Just the default value for libata.noacpi is changed to 1, so user can easily reenable it by passing boot/module parameter. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SATA Problem: port is slow to respond
Tejun Heo htejun at gmail.com writes: Sigmund Scheinbar wrote: Tejun Heo wrote: Please apply the attached patch over 2.6.20.1 and report how it works. I'm afraid that something went wrong while patching: Sorry, it was generated against the wrong tree. Regenerated patch attached. BTW.: I'm just curious, what's the purpose of this patch, it looks to me like it is just going to ignore the problem? Or do you think that the hardware is faulty? ATAPI devices use error conditions regularly to report all sorts of things including media not present. In some variants of ahci's, this sets seemingly unrelated error bits - JMBs set interface error and sb600s seem to set internal error. These errors when interpreted literally require device reset to recover from thus ATAPI devices never get through device ready check. So, the patch makes ahci driver suppress those spurious errors. I had the same problem with kernel 2.6.20.3 (from kernel.org) on the same machine (HP dc5750). I applied your patch, and the cd drive now works! dmesg shows no errors logged, and I can access the drive without any problem. Thanks, Rod. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 09:04:28 Thomas Gleixner wrote: On Tue, 2007-03-27 at 01:46 +0800, Jeff Chua wrote: On 3/27/07, Thomas Gleixner [EMAIL PROTECTED] wrote: It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can suspend and resume from RAM (s2ram). Even better, it works with/without CONFIG_NO_HZ. Does the patch below fix the HPET_TIMER=y case ? Thomas, I tried, but it didn't help. Upon resume from ram, date still didn't advance. Can you please issue a SysRq-Q in this situation and provide the dmesg output ? Thanks, tglx - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ Hi, I almost sure Iknow why this happens, The problem is that both hpet clock source and hpet clockevents doesn't have a suspend/resume function On resume we should enable the main counter _and_ enable legacy replacement mode, On my system main counter in enabled, by I think by bios, but legacy replacement mode is not, so if a system doesn't use lapic as a tick source, but use hpet+broadcast, it will hang for sure on resume, and i tested it The patch below is a temporally fix, until clock-events and clocksources will get proper suspend/resume hooks: Regards, Maxim Levitsky --- Add suspend/resume for HPET Signed-off-by: Maxim Levitsky [EMAIL PROTECTED] --- diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c index 0fd9fba..a1ec79e 100644 --- a/arch/i386/kernel/hpet.c +++ b/arch/i386/kernel/hpet.c @@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode, unsigned long cfg, cmp, now; uint64_t delta; + + if ( mode != CLOCK_EVT_MODE_UNUSED mode != CLOCK_EVT_MODE_SHUTDOWN) + { + unsigned long cfg = hpet_readl(HPET_CFG); + cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY; + hpet_writel(cfg, HPET_CFG); + + } + + switch(mode) { case CLOCK_EVT_MODE_PERIODIC: delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult; - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH pata-2.6] sl82c105: rework PIO support (take 2)
Get rid of the 'pio_speed' member of 'ide_drive_t' that was only used by this driver by storing the PIO mode timings in the 'drive_data' instead -- this allows us to greatly simplify the process of reloading of the chip's timing register and do it right in sl82c150_dma_off_quietly() and to get rid of two extra arguments to config_for_pio() -- which got renamed to sl82c105_tune_pio() and now returns a PIO mode selected, with ide_config_drive_speed() call moved into the tuneproc() method, now called sl82c105_tune_drive() with the code to set drive's 'io_32bit' and 'unmask' flags in its turn moved to its proper place in the init_hwif() method. Also, while at it, rename get_timing_sl82c105() into get_pio_timings() and get rid of the code in it clamping cycle counts to 32 which was both incorrect and never executed anyway... Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED] Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] --- The patch has been actually tested at last. :-) drivers/ide/pci/sl82c105.c | 99 ++--- include/linux/ide.h|1 2 files changed, 41 insertions(+), 59 deletions(-) Index: linux-2.6/drivers/ide/pci/sl82c105.c === --- linux-2.6.orig/drivers/ide/pci/sl82c105.c +++ linux-2.6/drivers/ide/pci/sl82c105.c @@ -11,6 +11,8 @@ * Merge in Russell's HW workarounds, fix various problems * with the timing registers setup. * -- Benjamin Herrenschmidt (01/11/03) [EMAIL PROTECTED] + * + * Copyright (C) 2006-2007 MontaVista Software, Inc. [EMAIL PROTECTED] */ #include linux/types.h @@ -47,25 +49,19 @@ #define CTRL_P0EN (1 0) /* - * Convert a PIO mode and cycle time to the required on/off - * times for the interface. This has protection against run-away - * timings. + * Convert a PIO mode and cycle time to the required on/off times + * for the interface. This has protection against runaway timings. */ -static unsigned int get_timing_sl82c105(ide_pio_data_t *p) +static unsigned int get_pio_timings(ide_pio_data_t *p) { - unsigned int cmd_on; - unsigned int cmd_off; + unsigned int cmd_on, cmd_off; - cmd_on = (ide_pio_timings[p-pio_mode].active_time + 29) / 30; + cmd_on = (ide_pio_timings[p-pio_mode].active_time + 29) / 30; cmd_off = (p-cycle_time - 30 * cmd_on + 29) / 30; - if (cmd_on 32) - cmd_on = 32; if (cmd_on == 0) cmd_on = 1; - if (cmd_off 32) - cmd_off = 32; if (cmd_off == 0) cmd_off = 1; @@ -73,44 +69,34 @@ static unsigned int get_timing_sl82c105( } /* - * Configure the drive and chipset for PIO + * Configure the chipset for PIO mode. */ -static void config_for_pio(ide_drive_t *drive, int pio, int report, int chipset_only) +static u8 sl82c105_tune_pio(ide_drive_t *drive, u8 pio) { - ide_hwif_t *hwif = HWIF(drive); - struct pci_dev *dev = hwif-pci_dev; + struct pci_dev *dev = HWIF(drive)-pci_dev; + int reg = 0x44 + drive-dn * 4; ide_pio_data_t p; - u16 drv_ctrl = 0x909; - unsigned int xfer_mode, reg; + u16 drv_ctrl; - DBG((config_for_pio(drive:%s, pio:%d, report:%d, chipset_only:%d)\n, - drive-name, pio, report, chipset_only)); - - reg = (hwif-channel ? 0x4c : 0x44) + (drive-select.b.unit ? 4 : 0); + DBG((sl82c105_tune_pio(drive:%s, pio:%u)\n, drive-name, pio)); pio = ide_get_best_pio_mode(drive, pio, 5, p); - xfer_mode = XFER_PIO_0 + pio; + drive-drive_data = drv_ctrl = get_pio_timings(p); - if (chipset_only || ide_config_drive_speed(drive, xfer_mode) == 0) { - drv_ctrl = get_timing_sl82c105(p); - drive-pio_speed = xfer_mode; - } else - drive-pio_speed = XFER_PIO_0; - - if (drive-using_dma == 0) { + if (!drive-using_dma) { /* * If we are actually using MW DMA, then we can not * reprogram the interface drive control register. */ - pci_write_config_word(dev, reg, drv_ctrl); - pci_read_config_word(dev, reg, drv_ctrl); - - if (report) { - printk(%s: selected %s (%dns) (%04X)\n, drive-name, - ide_xfer_verbose(xfer_mode), p.cycle_time, drv_ctrl); - } + pci_write_config_word(dev, reg, drv_ctrl); + pci_read_config_word (dev, reg, drv_ctrl); } + + printk(KERN_DEBUG %s: selected %s (%dns) (%04X)\n, drive-name, + ide_xfer_verbose(pio + XFER_PIO_0), p.cycle_time, drv_ctrl); + + return pio; } /* @@ -267,14 +253,14 @@ static int sl82c105_ide_dma_on (ide_driv static void sl82c105_dma_off_quietly(ide_drive_t *drive) { - u8 speed = XFER_PIO_0; + struct pci_dev
Re: libata-acpi: summary, problems, questions and proposal
On Wed, Mar 28, 2007 at 04:30:02PM +0900, Tejun Heo wrote: Hi Tejun, Firstly, could I ask you to take a look at the patch in http://permalink.gmane.org/gmane.linux.acpi.devel/22066/ ? It deals with some of these issues. ACPI support implementation in libata-dev supports both IDE and SATA ACPI object layouts and subset of ATA ACPI methods - _SDD and _GTF. It incorrectly uses ap-cbl (the port's cable type) to choose between the two ACPI layouts. Association between the host and its ACPI object is performed every time ACPI methods are invoked but the association between an ATA device and its ACPI object is cached in ata_device object. These issues are both fixed in my patch, I believe. 2-2. Missing proper _GTM/_STM support. As stated above, although -mm contains _GTM/_STM support, it does not hook it to regular exception handling path and thus _GTF cannot be used in a lot of cases. I've added _GTM and _STM support over suspend/resume. Right now they're in the host power management code - I'm not sure whether they should be here or the SCSI glue layer? 2-3. Misplaced _GTF hook. _GTF currently is called prior to every device configuration. This is unnecessary and incorrect. The ACPI spec specifies that _GTM/_STM and _GTF should be executed during suspend/resume cycles not on every reset or reconfiguration. This, for example, causes the following problem. That should be quite easily fixable with the above patch. 4-1. Depending on how questions in section 3 are answered, fix and clean up ATA host/device - ACPI object association. Whether IDE or SATA native style hierarchy is used should be determined by driver flag not cable type. e.g. ahci and sata_sil24 should use SATA native style hierarchy while ata_piix should use IDE hierarchy whether the port is SATA or PATA. I think this is just a matter of making sure that the sata and pata handle matching code matches reality now :) 4-2. Only associate once during initialization. There is no reason to try to associate hosts and devices with ACPI objects at each try. Do it once during host initialization and use it if available or forget about ACPI if not available. Fixed. 4-3. Integrate _GTM/_STM support and invoke methods only when the spec specifies to. For IDE object, do _GTM during suspend and _STM followed by _GTF during resume. There is no reason to call them anytime else. For SATA native object, do _SDD followed by _GTF after every hardreset. Patrially fixed. 4-4. Implement helpers for cable detection using _GTM/_STM and use it in sata_nv if CK804. This is to substitute independent pata_acpi driver. Low level driver should know when _GTM/_STM should be used for cable detection and/or device programming and doing it this way reduces user confusion (sata_nv also supports ck804 but you probably need to load pata_acpi if ACPI is available) and allows better integration with the rest of the low level driver (e.g. ADMA mode + _GTM/_STM cable detection). Not done. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH pata-2.6] sl82c105: rework PIO support (take 2)
Hello, I wrote: Index: linux-2.6/include/linux/ide.h === --- linux-2.6.orig/include/linux/ide.h +++ linux-2.6/include/linux/ide.h @@ -613,7 +613,6 @@ typedef struct ide_drive_s { u8 quirk_list; /* considered quirky, set for a specific host */ u8 init_speed; /* transfer rate set at boot */ -u8 pio_speed; /* unused by core, used by some drivers for fallback from DMA */ u8 current_speed; /* current transfer rate set */ u8 dn; /* now wide spread use */ u8 wcache; /* status of write cache */ Oops, forgot to pull the recent kernel before sending out, so this part won't apply bacause of the desired_speed nuisance. Do I need to resend? MBR, Sergei - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH pata-2.6] sl82c105: DMA support code cleanup (take 4)
Fold the now equivalent code in the ide_dma_check() method into a mere call to ide_use_dma(). Make config_for_dma() return non-zero if DMA mode has been set and call it from the ide_dma_check() method instead of ide_dma_on(). Defer writing the DMA timings to the chip registers until DMA is really turned on (and do not enable IORDY for DMA). Remove unneeded code from the init_hwif() method, improve its overall looks. Rename the dma_start(), ide_dma_check(), and ide_dma_lostirq() methods, and also use more proper hwif-dma_command, fix printk() and comment in the latter one as well. While at it, cleanup style in several places. --- This patch has also been actually tested at last. :-) Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED] Signed-off-by: Bartlomiej Zolnierkiewicz [EMAIL PROTECTED] drivers/ide/pci/sl82c105.c | 148 + 1 files changed, 59 insertions(+), 89 deletions(-) Index: linux-2.6/drivers/ide/pci/sl82c105.c === --- linux-2.6.orig/drivers/ide/pci/sl82c105.c +++ linux-2.6/drivers/ide/pci/sl82c105.c @@ -100,59 +100,28 @@ static u8 sl82c105_tune_pio(ide_drive_t } /* - * Configure the drive and the chipset for DMA + * Configure the drive for DMA. + * We'll program the chipset only when DMA is actually turned on. */ -static int config_for_dma (ide_drive_t *drive) +static int config_for_dma(ide_drive_t *drive) { - ide_hwif_t *hwif = HWIF(drive); - struct pci_dev *dev = hwif-pci_dev; - unsigned int reg; - DBG((config_for_dma(drive:%s)\n, drive-name)); - reg = (hwif-channel ? 0x4c : 0x44) + (drive-select.b.unit ? 4 : 0); - if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0) - return 1; - - pci_write_config_word(dev, reg, 0x0240); + return 0; - return 0; + return ide_dma_enable(drive); } /* - * Check to see if the drive and - * chipset is capable of DMA mode + * Check to see if the drive and chipset are capable of DMA mode. */ - -static int sl82c105_check_drive (ide_drive_t *drive) +static int sl82c105_ide_dma_check(ide_drive_t *drive) { - ide_hwif_t *hwif= HWIF(drive); - - DBG((sl82c105_check_drive(drive:%s)\n, drive-name)); + DBG((sl82c105_ide_dma_check(drive:%s)\n, drive-name)); - do { - struct hd_driveid *id = drive-id; - - if (!drive-autodma) - break; - - if (!id || !(id-capability 1)) - break; - - /* Consult the list of known bad drives */ - if (__ide_dma_bad_drive(drive)) - break; - - if (id-field_valid 2) { - if ((id-dma_mword hwif-mwdma_mask) || - (id-dma_1word hwif-swdma_mask)) - return 0; - } - - if (__ide_dma_good_drive(drive) id-eide_dma_time 150) - return 0; - } while (0); + if (ide_use_dma(drive) config_for_dma(drive)) + return 0; return -1; } @@ -181,14 +150,14 @@ static inline void sl82c105_reset_host(s * This function is called when the IDE timer expires, the drive * indicates that it is READY, and we were waiting for DMA to complete. */ -static int sl82c105_ide_dma_lost_irq(ide_drive_t *drive) +static int sl82c105_ide_dma_lostirq(ide_drive_t *drive) { - ide_hwif_t *hwif = HWIF(drive); - struct pci_dev *dev = hwif-pci_dev; - u32 val, mask = hwif-channel ? CTRL_IDE_IRQB : CTRL_IDE_IRQA; - unsigned long dma_base = hwif-dma_base; + ide_hwif_t *hwif= HWIF(drive); + struct pci_dev *dev = hwif-pci_dev; + u32 val, mask = hwif-channel ? CTRL_IDE_IRQB : CTRL_IDE_IRQA; + u8 dma_cmd; - printk(sl82c105: lost IRQ: resetting host\n); + printk(sl82c105: lost IRQ, resetting host\n); /* * Check the raw interrupt from the drive. @@ -201,15 +170,15 @@ static int sl82c105_ide_dma_lost_irq(ide * Was DMA enabled? If so, disable it - we're resetting the * host. The IDE layer will be handling the drive for us. */ - val = inb(dma_base); - if (val 1) { - outb(val ~1, dma_base); + dma_cmd = inb(hwif-dma_command); + if (dma_cmd 1) { + outb(dma_cmd ~1, hwif-dma_command); printk(sl82c105: DMA was enabled\n); } sl82c105_reset_host(dev); - /* ide_dmaproc would return 1, so we do as well */ + /* __ide_dma_lostirq would return 1, so we do as well */ return 1; } @@ -221,10 +190,10 @@ static int sl82c105_ide_dma_lost_irq(ide * The generic IDE core will have disabled the BMEN bit before this * function is called. */ -static void sl82c105_ide_dma_start(ide_drive_t *drive) +static void
[PATCH pata-2.6] sl82c105: add speedproc() method and MWDMA0/1 support
Add the speedproc() method for setting transfer modes, modify config_for_dma() to call it and use ide_max_dma_mode() to select the best DMA mode. Add support for the multiword DMA modes 0 and 1, using the upper half of the 'drive_data' field to store the DMA timings to program into the drive control register when DMA is turned on for real. Signed-off-by: Sergei Shtylyov [EMAIL PROTECTED] --- This patch replaces sl82c105-add-speedproc.patch from pata-2.6 patchset. drivers/ide/pci/sl82c105.c | 71 + 1 files changed, 66 insertions(+), 5 deletions(-) Index: linux-2.6/drivers/ide/pci/sl82c105.c === --- linux-2.6.orig/drivers/ide/pci/sl82c105.c +++ linux-2.6/drivers/ide/pci/sl82c105.c @@ -82,7 +82,14 @@ static u8 sl82c105_tune_pio(ide_drive_t pio = ide_get_best_pio_mode(drive, pio, 5, p); - drive-drive_data = drv_ctrl = get_pio_timings(p); + drv_ctrl = get_pio_timings(p); + + /* +* Store the PIO timings so that we can restore them +* in case DMA will be turned off... +*/ + drive-drive_data = 0x; + drive-drive_data |= drv_ctrl; if (!drive-using_dma) { /* @@ -100,14 +107,67 @@ static u8 sl82c105_tune_pio(ide_drive_t } /* + * Configure the drive and chipset for a new transfer speed. + */ +static int sl82c105_tune_chipset(ide_drive_t *drive, u8 speed) +{ + static u16 mwdma_timings[] = {0x0707, 0x0201, 0x0200}; + u16 drv_ctrl; + + DBG((sl82c105_tune_chipset(drive:%s, speed:%s)\n, +drive-name, ide_xfer_verbose(speed))); + + speed = ide_rate_filter(drive, speed); + + switch (speed) { + case XFER_MW_DMA_2: + case XFER_MW_DMA_1: + case XFER_MW_DMA_0: + drv_ctrl = mwdma_timings[speed - XFER_MW_DMA_0]; + + /* +* Store the DMA timings so that we can actually program +* them when DMA will be turned on... +*/ + drive-drive_data = 0x; + drive-drive_data |= (unsigned long)drv_ctrl 16; + + /* +* If we are already using DMA, we just reprogram +* the drive control register. +*/ + if (drive-using_dma) { + struct pci_dev *dev = HWIF(drive)-pci_dev; + int reg = 0x44 + drive-dn * 4; + + pci_write_config_word(dev, reg, drv_ctrl); + } + break; + case XFER_PIO_5: + case XFER_PIO_4: + case XFER_PIO_3: + case XFER_PIO_2: + case XFER_PIO_1: + case XFER_PIO_0: + (void) sl82c105_tune_pio(drive, speed - XFER_PIO_0); + break; + default: + return -1; + } + + return ide_config_drive_speed(drive, speed); +} + +/* * Configure the drive for DMA. - * We'll program the chipset only when DMA is actually turned on. */ static int config_for_dma(ide_drive_t *drive) { + u8 speed = ide_max_dma_mode(drive); + DBG((config_for_dma(drive:%s)\n, drive-name)); - if (ide_config_drive_speed(drive, XFER_MW_DMA_2) != 0) + if (!speed || sl82c105_tune_chipset(drive, speed)) return 0; return ide_dma_enable(drive); @@ -219,7 +279,7 @@ static int sl82c105_ide_dma_on(ide_drive rc = __ide_dma_on(drive); if (rc == 0) { - pci_write_config_word(dev, reg, 0x0200); + pci_write_config_word(dev, reg, drive-drive_data 16); printk(KERN_INFO %s: DMA enabled\n, drive-name); } @@ -357,6 +417,7 @@ static void __devinit init_hwif_sl82c105 DBG((init_hwif_sl82c105(hwif: ide%d)\n, hwif-index)); hwif-tuneproc = sl82c105_tune_drive; + hwif-speedproc = sl82c105_tune_chipset; hwif-selectproc= sl82c105_selectproc; hwif-resetproc = sl82c105_resetproc; @@ -388,7 +449,7 @@ static void __devinit init_hwif_sl82c105 } hwif-atapi_dma = 1; - hwif-mwdma_mask = 0x04; + hwif-mwdma_mask = 0x07; hwif-ide_dma_check = sl82c105_ide_dma_check; hwif-ide_dma_on= sl82c105_ide_dma_on; - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Problems with VIA VT8251 SATA and kernels above 2.6.18.x
On Sunday 25 March 2007 19:27, Rüdiger Otte wrote: Dear Linux ATA people, the SATA-Controller on Asus A8V-MX mainboard (VIA VT8251) doesn't recognize an attached SATA-Harddisk (Western Digital WD3200AAKS in my case) with kernels 2.6.19 and above. I get the error with 2.6.19, 2.6.20 and 2.6.21-rc4, but when switching back to 2.6.18.8 everything works fine. Also there are several identical reports of this problem on VIA arena and other sites, so maybe someone could fix this. Appended is the kernel output of ahci-driver both with working and with non working kernels. You need to add pci=nomsi to your kernel boot line. -- Have you mooed today?... - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why is NCQ enabled by default by libata? (2.6.20)
Phillip Susi wrote: Justin Piszcz wrote: I would try with write-caching enabled. Also, the RAID5/RAID10 you mention seems like each volume is on part of the platter, a strange setup you got there :) Shouldn't NCQ only help write performance if write caching is _disabled_? Since write cache essentially is just non tagged command queuing? NCQ provides for a more asynchronous flow. It helps greatly with reads (of which most are, by nature, synchronous at the app level) from multiple threads or apps. It helps with writes, even with write cache on, by allowing multiple commands to be submitted and/or retired at the same time. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why is NCQ enabled by default by libata? (2.6.20)
Justin Piszcz wrote: I would try with write-caching enabled. Also, the RAID5/RAID10 you mention seems like each volume is on part of the platter, a strange setup you got there :) Shouldn't NCQ only help write performance if write caching is _disabled_? Since write cache essentially is just non tagged command queuing? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [2/5] 2.6.21-rc5: known regressions
Le 27.03.2007 03:59, Adrian Bunk a écrit : This email lists some known regressions in Linus' tree compared to 2.6.20. ... Subject: libata: PATA UDMA/100 configured as UDMA/33 References : http://lkml.org/lkml/2007/2/20/294 http://www.mail-archive.com/linux-ide@vger.kernel.org/msg04115.html http://bugzilla.kernel.org/show_bug.cgi?id=8133 http://bugzilla.kernel.org/show_bug.cgi?id=8164 http://lkml.org/lkml/2007/3/21/330 Submitter : Fabio Comolli [EMAIL PROTECTED] Plamen Petrov [EMAIL PROTECTED] Laurent Riffard [EMAIL PROTECTED] Lukas Hejtmanek [EMAIL PROTECTED] Handled-By : Tejun Heo [EMAIL PROTECTED] Patch : http://thread.gmane.org/gmane.linux.ide/17444 Status : patch available pata-via case is fixed for me in 2.6.21-rc5-mm2 (was already fixed in 2.6.21-rc4-mm1). thanks ~~ laurent - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA patches
On Wed, Mar 28, 2007 at 10:57:54AM +0100, Alan Cox wrote: Ok thanks. This is interesting as the only thing new it is doing is asking for the HPA size. Does I think explain the problem however: Can you move the HPA setting call to after the mode has been set and see if that makes the problem vanish ? Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than sd: 2:0:1:0: timing out command, waited 180s sd: 2:0:1:0: SCSI error: return code = 0x0028 end_request: I/O error, dev sda, sector 0 Buffer I/O error on device sda, logical block 0 -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA patches
Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means no HPA in this case I guess ? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata fixes
On Wed, 28 Mar 2007, Jeff Garzik wrote: Jeff Garzik wrote: This disables libata ACPI, among other things. If a -rc6 is possible, that would be quite nice... Heh. I don't think -rc6 is possible - it's inevitable. We have too much fallout from the timer changes still outstanding. It looks people finally figured out a big issue with the HPET timer, and that hopefully resolves most of the remaining timer-related regressions, but yes, we most definitely _will_ have an -rc6. Andrew, what's your feeling apart from the timer fallout? Linus - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata fixes
On Wed, 28 Mar 2007 13:53:10 -0700 (PDT) Linus Torvalds [EMAIL PROTECTED] wrote: On Wed, 28 Mar 2007, Jeff Garzik wrote: Jeff Garzik wrote: This disables libata ACPI, among other things. If a -rc6 is possible, that would be quite nice... Heh. I don't think -rc6 is possible - it's inevitable. We have too much fallout from the timer changes still outstanding. It looks people finally figured out a big issue with the HPET timer, and that hopefully resolves most of the remaining timer-related regressions, but yes, we most definitely _will_ have an -rc6. yup. Andrew, what's your feeling apart from the timer fallout? There are two main metrics: a) the number of bugs which Adrian is tracking. I think this still exceeds 25, which is a lot. b) the rate at which fixes are arriving. I have accumulated 15-20 since the last batch (40 hours ago), which is still a pretty high rate. Based on that, we're still quite a long way from -final. (But you know me - I'd be happy releasing 2.6.21 in July) (Don't ask me what year I'm referring to, either) There is another metric to look at, too: the number of fixes which are going into 2.6.x.y. If that fix count is high, and if those fixes fix bugs which were not present in 2.6.x-1 then this is an indication that something is wrong - many regressions are sneaking through the -rc process. And I haven't run the numbers, but I get the impression that 2.6.20.x has an unusually large number of fixes in it. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HPA patches
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote: Hm. I tried adding it in the eh code after ata_set_mode() in ata_eh_recover(), which alters the problem slightly - hpa_sectors is still 0, so the taskfile call is still failing, but now the system just stops at around the time that anything attempts to access sda with no errors other than I wonder if the firmware is dying when we ask the disk to go zero sized rather than erroring politely. I'm not sure hth HPA sectors can come back as zero but we can be fairly sure 0 means no HPA in this case I guess ? The command is Read Native Max Sectors which should be the full disk size as long as the command is supported, and the size returned by IDENTIFY would be smaller if HPA was in use. AIUI at least. Cheers, Kyle - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/3] Asynchronous Notification for SATA ATAPI devices
This patch series implements Asynchronous Notification (AN) for SATA ATAPI devices as defined in SATA 2.5 and AHCI 1.1. Drives which support this feature will send a notification when new media is inserted into the drive, preventing the need for user space to poll for new media. This support is exposed to user space via a file in sysfs (/sys/block/sr*) called media_change_events. If the drive supports AN, this file will read 1, otherwise 0. User space can disable polling for new media if this file reads 1. When new media is inserted into the ATAPI drive, the ahci driver will send a KOBJ_CHANGE event. I would really like feedback on the user interface - both the location of the sysfs file which indicates AN support, as well as the type of uevent etc. I have not yet tested AN on eject (I assume it doesn't require anything special) as my test drive which supports AN is a bit quirky in this respect. Please take a look and let me know what you think. Thanks, Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/3] libata: expose AN support to user space via sysfs
Allow user space to determine if an ATAPI device supports async notification (AN) of media changes. This is done by adding a new sysfs file async_notification to genhd. If the file reads 1, then the device supports async notification. If the file reads 0, it does not. A flag is set in the generic disk to indicate whether or not AN is supported. This flag is set by the SCSI subsystem when it registers with add_disk. The SCSI system gets information from libata on whether the device supports AN during dev_configure. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-mm/block/genhd.c === --- 2.6-mm.orig/block/genhd.c +++ 2.6-mm/block/genhd.c @@ -372,6 +372,11 @@ static ssize_t disk_size_read(struct gen { return sprintf(page, %llu\n, (unsigned long long)get_capacity(disk)); } +static ssize_t disk_AN_read(struct gendisk * disk, char *page) +{ + return sprintf(page, %d\n, + (disk-flags GENHD_FL_ASYNC_NOTIFICATION ? 1 : 0)); +} static ssize_t disk_stats_read(struct gendisk * disk, char *page) { @@ -419,6 +424,10 @@ static struct disk_attribute disk_attr_s .attr = {.name = stat, .mode = S_IRUGO }, .show = disk_stats_read }; +static struct disk_attribute disk_attr_AN = { + .attr = {.name = media_change_events, .mode = S_IRUGO }, + .show = disk_AN_read +}; #ifdef CONFIG_FAIL_MAKE_REQUEST @@ -455,6 +464,7 @@ static struct attribute * default_attrs[ disk_attr_removable.attr, disk_attr_size.attr, disk_attr_stat.attr, + disk_attr_AN.attr, #ifdef CONFIG_FAIL_MAKE_REQUEST disk_attr_fail.attr, #endif Index: 2.6-mm/include/linux/genhd.h === --- 2.6-mm.orig/include/linux/genhd.h +++ 2.6-mm/include/linux/genhd.h @@ -94,6 +94,7 @@ struct hd_struct { #define GENHD_FL_REMOVABLE 1 #define GENHD_FL_DRIVERFS 2 +#define GENHD_FL_ASYNC_NOTIFICATION4 #define GENHD_FL_CD8 #define GENHD_FL_UP16 #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 Index: 2.6-mm/include/scsi/scsi_device.h === --- 2.6-mm.orig/include/scsi/scsi_device.h +++ 2.6-mm/include/scsi/scsi_device.h @@ -126,7 +126,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned async_notification:1; /* device supports async notification */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ Index: 2.6-mm/drivers/ata/libata-scsi.c === --- 2.6-mm.orig/drivers/ata/libata-scsi.c +++ 2.6-mm/drivers/ata/libata-scsi.c @@ -899,6 +899,9 @@ static void ata_scsi_dev_config(struct s blk_queue_max_hw_segments(q, q-max_hw_segments - 1); } + if (dev-flags ATA_DFLAG_AN) + sdev-async_notification = 1; + if (dev-flags ATA_DFLAG_NCQ) { int depth; Index: 2.6-mm/drivers/scsi/sr.c === --- 2.6-mm.orig/drivers/scsi/sr.c +++ 2.6-mm/drivers/scsi/sr.c @@ -603,6 +603,8 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-async_notification) + disk-flags |= GENHD_FL_ASYNC_NOTIFICATION; add_disk(disk); sdev_printk(KERN_DEBUG, sdev, -- - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/3] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-mm/drivers/ata/libata-core.c === --- 2.6-mm.orig/drivers/ata/libata-core.c +++ 2.6-mm/drivers/ata/libata-core.c @@ -71,6 +71,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); static unsigned int ata_print_id = 1; @@ -1745,6 +1746,23 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if (ata_id_has_AN(id)) + { + /* issue SET feature command to turn this on */ + rc = ata_dev_set_AN(dev); + if (rc) { + ata_dev_printk(dev, KERN_ERR, + unable to set AN\n); + rc = -EINVAL; + goto err_out_nosup; + } + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3642,6 +3660,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-mm/include/linux/ata.h === --- 2.6-mm.orig/include/linux/ata.h +++ 2.6-mm/include/linux/ata.h @@ -193,6 +193,12 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -298,6 +304,8 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ((id[76] (~id[76])) ((id)[78] (1 5))) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-mm/include/linux/libata.h === --- 2.6-mm.orig/include/linux/libata.h +++ 2.6-mm/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ -- - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at
[patch 3/3] libata: handle AN interrupt
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. The ahci host controller will send the event via KOBJ_CHANGE uevent. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-mm/drivers/ata/ahci.c === --- 2.6-mm.orig/drivers/ata/ahci.c +++ 2.6-mm/drivers/ata/ahci.c @@ -1164,6 +1164,26 @@ static void ahci_host_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev = ap-device[port_addr]; + ata_port_printk(ap, KERN_INFO, N bit set on SDB FIS!\n); + if (adev-flags ATA_DFLAG_AN) + ata_async_notify(adev); + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-mm/include/linux/libata.h === --- 2.6-mm.orig/include/linux/libata.h +++ 2.6-mm/include/linux/libata.h @@ -492,6 +492,7 @@ struct ata_device { /* ACPI objects info */ acpi_handle obj_handle; #endif + struct work_struct async_notify; }; /* Offset into struct ata_device. Fields above it are maintained @@ -826,6 +827,7 @@ extern void ata_scsi_slave_destroy(struc extern int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth); extern struct ata_device *ata_dev_pair(struct ata_device *adev); +extern void ata_async_notify(struct ata_device *atadev); extern int ata_do_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev); extern u8 ata_irq_on(struct ata_port *ap); extern u8 ata_dummy_irq_on(struct ata_port *ap); Index: 2.6-mm/drivers/ata/libata-core.c === --- 2.6-mm.orig/drivers/ata/libata-core.c +++ 2.6-mm/drivers/ata/libata-core.c @@ -1576,6 +1576,26 @@ static void ata_dev_config_ncq(struct at snprintf(desc, desc_sz, NCQ (depth %d/%d), hdepth, ddepth); } +static void async_notify_thread(struct work_struct *work) +{ + struct ata_device *atadev = + container_of(work, struct ata_device, async_notify); + + /* +* TBD - who should send this event? I couldn't find an +* easy way to map an ata_device to a genhd device, so +* decided maybe the ata host should send the event and +* allow user space to figure out what happened? +*/ + kobject_uevent(atadev-ap-host-dev-kobj, KOBJ_CHANGE); +} + +void ata_async_notify(struct ata_device *atadev) +{ + schedule_work(atadev-async_notify); +} + + /** * ata_dev_configure - Configure the specified ATA/ATAPI device * @dev: Target device to configure @@ -1761,6 +1781,7 @@ int ata_dev_configure(struct ata_device goto err_out_nosup; } dev-flags |= ATA_DFLAG_AN; + INIT_WORK(dev-async_notify, async_notify_thread); } if (ata_id_cdb_intr(dev-id)) { @@ -6650,6 +6671,7 @@ EXPORT_SYMBOL_GPL(ata_dummy_irq_on); EXPORT_SYMBOL_GPL(ata_irq_ack); EXPORT_SYMBOL_GPL(ata_dummy_irq_ack); EXPORT_SYMBOL_GPL(ata_dev_try_classify); +EXPORT_SYMBOL_GPL(ata_async_notify); EXPORT_SYMBOL_GPL(ata_cable_40wire); EXPORT_SYMBOL_GPL(ata_cable_80wire); -- - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/3] libata: expose AN support to user space via sysfs
Kristen Carlson Accardi wrote: Allow user space to determine if an ATAPI device supports async notification (AN) of media changes. This is done by adding a new sysfs file async_notification to genhd. If the file reads 1, then the device supports async notification. If the file reads 0, it does not. A flag is set in the generic disk to indicate whether or not AN is supported. This flag is set by the SCSI subsystem when it registers with add_disk. The SCSI system gets information from libata on whether the device supports AN during dev_configure. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-mm/block/genhd.c === --- 2.6-mm.orig/block/genhd.c +++ 2.6-mm/block/genhd.c @@ -372,6 +372,11 @@ static ssize_t disk_size_read(struct gen { return sprintf(page, %llu\n, (unsigned long long)get_capacity(disk)); } +static ssize_t disk_AN_read(struct gendisk * disk, char *page) +{ + return sprintf(page, %d\n, + (disk-flags GENHD_FL_ASYNC_NOTIFICATION ? 1 : 0)); +} static ssize_t disk_stats_read(struct gendisk * disk, char *page) { @@ -419,6 +424,10 @@ static struct disk_attribute disk_attr_s .attr = {.name = stat, .mode = S_IRUGO }, .show = disk_stats_read }; +static struct disk_attribute disk_attr_AN = { + .attr = {.name = media_change_events, .mode = S_IRUGO }, + .show = disk_AN_read +}; #ifdef CONFIG_FAIL_MAKE_REQUEST @@ -455,6 +464,7 @@ static struct attribute * default_attrs[ disk_attr_removable.attr, disk_attr_size.attr, disk_attr_stat.attr, + disk_attr_AN.attr, #ifdef CONFIG_FAIL_MAKE_REQUEST disk_attr_fail.attr, #endif Index: 2.6-mm/include/linux/genhd.h === --- 2.6-mm.orig/include/linux/genhd.h +++ 2.6-mm/include/linux/genhd.h @@ -94,6 +94,7 @@ struct hd_struct { #define GENHD_FL_REMOVABLE 1 #define GENHD_FL_DRIVERFS 2 +#define GENHD_FL_ASYNC_NOTIFICATION4 #define GENHD_FL_CD8 #define GENHD_FL_UP16 #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 Index: 2.6-mm/include/scsi/scsi_device.h === --- 2.6-mm.orig/include/scsi/scsi_device.h +++ 2.6-mm/include/scsi/scsi_device.h @@ -126,7 +126,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned async_notification:1; /* device supports async notification */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ Index: 2.6-mm/drivers/ata/libata-scsi.c === --- 2.6-mm.orig/drivers/ata/libata-scsi.c +++ 2.6-mm/drivers/ata/libata-scsi.c @@ -899,6 +899,9 @@ static void ata_scsi_dev_config(struct s blk_queue_max_hw_segments(q, q-max_hw_segments - 1); } + if (dev-flags ATA_DFLAG_AN) + sdev-async_notification = 1; + if (dev-flags ATA_DFLAG_NCQ) { int depth; Index: 2.6-mm/drivers/scsi/sr.c === --- 2.6-mm.orig/drivers/scsi/sr.c +++ 2.6-mm/drivers/scsi/sr.c @@ -603,6 +603,8 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-async_notification) + disk-flags |= GENHD_FL_ASYNC_NOTIFICATION; add_disk(disk); (added linux-scsi to CC) Comments: 1) From a procedural standpoint, you'll want to separate this patch into three patches: generic block layer stuff, SCSI stuff, and libata stuff. 2) I don't claim to be a sysfs expert, but this seems like a reasonable approach for reporting async-notification capabilities 3) I would make the contents of 'media_change_events' be a list of flags, rather than a boolean. Thus, when AN is present, media_change_events would return AN\n. It would return \n (no flags) when AN is absent. This permits future expansion of this capabilities reporting variable. 4) Figure out some place to document 'media_change_events', in Documentation/* 5) I think the method of delivery probably needs discussing, and some work. Presumably the normal hotplug paths should be traversed for this sort of thing. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
* Maxim [EMAIL PROTECTED] wrote: I almost sure I know why this happens, cool! Your patch is a definite improvement on my t60 (where suspend/resume never worked with hpet enabled). But it does not fix everything - for example the timings are way off after resume. Thomas? Ingo - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
On Wed, 28 Mar 2007, Maxim wrote: Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources don't have _any_ resume hook. One thing that drives me wild about that clocksource resume thing is that it seems to think that clocksources are somehow different from any other system devices.. Why isn't the HPET considered a device, and has it's own *device* suspend and resume? Why do we seem to think that only set_mode() etc should wake up clock sources? It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Thomas? It does seem like Maxim has hit the nail on the head (at least partly) on the HPET timer resume problems.. Linus - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
Subject: first disk access after resume takes several minutes ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n) References : http://lkml.org/lkml/2007/3/8/117 http://lkml.org/lkml/2007/3/25/20 Submitter : Michael S. Tsirkin [EMAIL PROTECTED] ... Subject: after resume: X hangs after drawing a couple of windows References : http://lkml.org/lkml/2007/3/8/117 Submitter : Michael S. Tsirkin [EMAIL PROTECTED] Status : unknown ... From: Jeff Chua [EMAIL PROTECTED] It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can suspend and resume from RAM (s2ram). Even better, it works with/without CONFIG_NO_HZ. Quoting Maxim [EMAIL PROTECTED]: Hi, I almost sure Iknow why this happens, The problem is that both hpet clock source and hpet clockevents doesn't have a suspend/resume function On resume we should enable the main counter _and_ enable legacy replacement mode, On my system main counter in enabled, by I think by bios, but legacy replacement mode is not, so if a system doesn't use lapic as a tick source, but use hpet+broadcast, it will hang for sure on resume, and i tested it The patch below is a temporally fix, until clock-events and clocksources will get proper suspend/resume hooks: Regards, Maxim Levitsky Bingo! The patch below fixes the two problems (listed above) with resume from RAM that I have observed on my T60 with 2.6.21-rc5: with this patch applied, and with CONFIG_NO_HZ unset, date advances correctly, X functions properly and there is no delay on first disk access. Thanks very much. --- Add suspend/resume for HPET Signed-off-by: Maxim Levitsky [EMAIL PROTECTED] Maxim, do you plan to send this upstream? Acked-by: Michael S. Tsirkin [EMAIL PROTECTED] --- diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c index 0fd9fba..a1ec79e 100644 --- a/arch/i386/kernel/hpet.c +++ b/arch/i386/kernel/hpet.c @@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode, unsigned long cfg, cmp, now; uint64_t delta; + + if ( mode != CLOCK_EVT_MODE_UNUSED mode != CLOCK_EVT_MODE_SHUTDOWN) + { + unsigned long cfg = hpet_readl(HPET_CFG); + cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY; + hpet_writel(cfg, HPET_CFG); + + } + + switch(mode) { case CLOCK_EVT_MODE_PERIODIC: delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult; -- MST - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
* Michael S. Tsirkin [EMAIL PROTECTED] wrote: Bingo! The patch below fixes the two problems (listed above) with resume from RAM that I have observed on my T60 with 2.6.21-rc5: with this patch applied, and with CONFIG_NO_HZ unset, date advances correctly, X functions properly and there is no delay on first disk access. Thanks very much. [...] Maxim, do you plan to send this upstream? we'll fix this the right way tomorrow, by adding a proper suspend/resume sysdev handler - the lapic clockevents driver already has that. Maxim definitely deserves alot of kudos for having found this bug! Ingo - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3/6] 2.6.21-rc4: known regressions
On Wed, 28 Mar 2007 20:04:57 +0200 Michael S. Tsirkin wrote: Subject: first disk access after resume takes several minutes ('date' does not advance after resume from RAM, CONFIG_NO_HZ=n) References : http://lkml.org/lkml/2007/3/8/117 http://lkml.org/lkml/2007/3/25/20 Submitter : Michael S. Tsirkin [EMAIL PROTECTED] ... Subject: after resume: X hangs after drawing a couple of windows References : http://lkml.org/lkml/2007/3/8/117 Submitter : Michael S. Tsirkin [EMAIL PROTECTED] Status : unknown ... From: Jeff Chua [EMAIL PROTECTED] It's related. I tested without CONFIG_HPET_TIMER, and now my X60 can suspend and resume from RAM (s2ram). Even better, it works with/without CONFIG_NO_HZ. Quoting Maxim [EMAIL PROTECTED]: Hi, I almost sure Iknow why this happens, The problem is that both hpet clock source and hpet clockevents doesn't have a suspend/resume function On resume we should enable the main counter _and_ enable legacy replacement mode, On my system main counter in enabled, by I think by bios, but legacy replacement mode is not, so if a system doesn't use lapic as a tick source, but use hpet+broadcast, it will hang for sure on resume, and i tested it The patch below is a temporally fix, until clock-events and clocksources will get proper suspend/resume hooks: Regards, Maxim Levitsky Bingo! The patch below fixes the two problems (listed above) with resume from RAM that I have observed on my T60 with 2.6.21-rc5: with this patch applied, and with CONFIG_NO_HZ unset, date advances correctly, X functions properly and there is no delay on first disk access. Thanks very much. --- Add suspend/resume for HPET Signed-off-by: Maxim Levitsky [EMAIL PROTECTED] Maxim, do you plan to send this upstream? with whitespace fixes, please... Acked-by: Michael S. Tsirkin [EMAIL PROTECTED] --- diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c index 0fd9fba..a1ec79e 100644 --- a/arch/i386/kernel/hpet.c +++ b/arch/i386/kernel/hpet.c @@ -152,6 +152,16 @@ static void hpet_set_mode(enum clock_event_mode mode, unsigned long cfg, cmp, now; uint64_t delta; + + if ( mode != CLOCK_EVT_MODE_UNUSED mode != CLOCK_EVT_MODE_SHUTDOWN) + { if (mode != CLOCK_EVT_MODE_UNUSED mode != CLOCK_EVT_MODE_SHUTDOWN) { + unsigned long cfg = hpet_readl(HPET_CFG); + cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY; + hpet_writel(cfg, HPET_CFG); + delete above line. + } + + switch(mode) { case CLOCK_EVT_MODE_PERIODIC: delta = ((uint64_t)(NSEC_PER_SEC/HZ)) * hpet_clockevent.mult; --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote: It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Agreed, except about probably as a system device. Last I checked, there was no good reason to use sysdev suspend()/resume() rather than platform_device suspend_late()/early_resume(). Which more or less means no good reason to use sysdev in new code... Also, making HPET use the legacy mode seems like a step backwards. - Dave - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 21:38:55 David Brownell wrote: On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote: It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Agreed, except about probably as a system device. Last I checked, there was no good reason to use sysdev suspend()/resume() rather than platform_device suspend_late()/early_resume(). Which more or less means no good reason to use sysdev in new code... Also, making HPET use the legacy mode seems like a step backwards. - Dave Hi, It is not 'legacy' mode, It is a legacy replacement mode. It this mode HPET takes over IRQ0 and IRQ 8 and provides this way replacement for PIT and RTC periodic function Best regards, Maxim Levitsky - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wed, 28 Mar 2007, David Brownell wrote: On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote: It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Agreed, except about probably as a system device. Last I checked, there was no good reason to use sysdev suspend()/resume() rather than platform_device suspend_late()/early_resume(). Which more or less means no good reason to use sysdev in new code... I won't disagree - it might well be much nicer to just show it in the real device tree. I'm not 100% sure where in the tree it would go, though. It should probably be inside the root entry, before any of the PCI buses. It's generally what we've used those system device things for, but I agree that it would be better to just make system devices show up early on the regular device list than it is to have them be special cases. Bit I think that's a separate (and fairly small) issue compared to the don't use the clocksource infrastructure as a make-believe suspend/resume mechanism problem that Maxim's patch had. (Maxim, don't take that the wrong way - I think your analysis and patch were great, I just think another organization would be better) Also, making HPET use the legacy mode seems like a step backwards. I don't think that's actually legacy in any sense but the interrupt delivery, where the legacy mode bit is not so much that the HPET itself is legacy but that it *replaces* legacy devices. But I may have misunderstood the thing. I'm an old fart, so I know the old timers much better than I know the new ones ;). Somebody feel free to hit me with the clue-2x4. Linus - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 1:19 pm, Maxim wrote: On Wednesday 28 March 2007 21:38:55 David Brownell wrote: Also, making HPET use the legacy mode seems like a step backwards. It is not 'legacy' mode, It is a legacy replacement mode. Typo, sorry. It this mode HPET takes over IRQ0 and IRQ 8 and provides this way replacement for PIT and RTC periodic function It's that RTC periodic thing that bothers me, I don't mind about the PIT. Remember that IRQ8 is also used for other RTC functions. Now, if there were a way to tell rtc-cmos that HPET is active, and arrange some kind of handshake ... that would be different. - Dave - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 1:42 pm, Linus Torvalds wrote: I won't disagree - it might well be much nicer to just show it in the real device tree. I'm not 100% sure where in the tree it would go, though. It should probably be inside the root entry, before any of the PCI buses. Mixing inside and before is a small linguistic clue about one of the issues with driver model PM. Off topic here; and in terms of suspend/resume callback sequencing that answer shouldn't matter much for HPET (as I understand things). It's generally what we've used those system device things for, but I agree that it would be better to just make system devices show up early on the regular device list than it is to have them be special cases. Yes -- where platform_device is a regular Joe-Sixpack kind of device, but sysdev is a special case. Bit I think that's a separate (and fairly small) issue compared to the don't use the clocksource infrastructure as a make-believe suspend/resume mechanism problem that Maxim's patch had. Agreed -- although isn't it the clockevent change which is at issue? A clockevent thingie wraps various kinds of timer IRQs; the clocksource is conceptually just a free run counter. Clocksources have been around for a while, with no particular problems. It's clockevent sources have been the problem with dynamic tick solutions all along, since they mask such chaos inside x86 hardware and interact with so many different parts of the kernel. ;) - Dave (Maxim, don't take that the wrong way - I think your analysis and patch were great, I just think another organization would be better) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 22:59:26 David Brownell wrote: On Wednesday 28 March 2007 1:19 pm, Maxim wrote: On Wednesday 28 March 2007 21:38:55 David Brownell wrote: Also, making HPET use the legacy mode seems like a step backwards. It is not 'legacy' mode, It is a legacy replacement mode. Typo, sorry. It this mode HPET takes over IRQ0 and IRQ 8 and provides this way replacement for PIT and RTC periodic function It's that RTC periodic thing that bothers me, I don't mind about the PIT. Remember that IRQ8 is also used for other RTC functions. Now, if there were a way to tell rtc-cmos that HPET is active, and arrange some kind of handshake ... that would be different. - Dave Yes, When HPET is active it eats RTC IRQ, So the only way out is to emulate RTC using HPET, It is done this way in old rtc driver, rtc-cmos should do the same. Of course suspend resume is not supported at all by old rtc driver I already wrote complete support for suspend/resume for old rtc driver (I wrote it long time ago) Now I fixed it to support HPET , and this way I discovered that HPET doesn't have suspend resume functions I will do last checks now and send this patch very soon I am also planning to add support of HPET and suspend/resume for rtc-cmos, but I didn't start this yet. Best regards, Maxim Levitsky - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-pm] [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 22:42:00 Linus Torvalds wrote: On Wed, 28 Mar 2007, David Brownell wrote: On Wednesday 28 March 2007 9:38 am, Linus Torvalds wrote: It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Agreed, except about probably as a system device. Last I checked, there was no good reason to use sysdev suspend()/resume() rather than platform_device suspend_late()/early_resume(). Which more or less means no good reason to use sysdev in new code... I won't disagree - it might well be much nicer to just show it in the real device tree. I'm not 100% sure where in the tree it would go, though. It should probably be inside the root entry, before any of the PCI buses. It's generally what we've used those system device things for, but I agree that it would be better to just make system devices show up early on the regular device list than it is to have them be special cases. Bit I think that's a separate (and fairly small) issue compared to the don't use the clocksource infrastructure as a make-believe suspend/resume mechanism problem that Maxim's patch had. (Maxim, don't take that the wrong way - I think your analysis and patch were great, I just think another organization would be better) Exactly, I agree completely I said that my patch was a temporary fix, and I agree that the best way is to create a new system device and use its suspend/resume hooks to bring HPET back to life on resume. Also, making HPET use the legacy mode seems like a step backwards. I don't think that's actually legacy in any sense but the interrupt delivery, where the legacy mode bit is not so much that the HPET itself is legacy but that it *replaces* legacy devices. But I may have misunderstood the thing. I'm an old fart, so I know the old timers much better than I know the new ones ;). Somebody feel free to hit me with the clue-2x4. Linus Best regards, Maxim Levitsky - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add notation that the Asus W5F laptop has a short cable instead of 80-wire.
The Asus W5F laptop uses a short cable instead of the 80-wire style, and thus needs to be in the ich_laptop special cases for correct detection and support of UDMA/100 for the hard drive. I noticed this because I have the W5F laptop, and was tracing apparent slowness. Signed-off-by: Robin H. Johnson [EMAIL PROTECTED] --- drivers/ata/ata_piix.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index b952c58..a2c5756 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -580,6 +580,7 @@ static const struct ich_laptop ich_laptop[] = { /* devid, subvendor, subdev */ { 0x27DF, 0x0005, 0x0280 }, /* ICH7 on Acer 5602WLMi */ { 0x27DF, 0x1025, 0x0110 }, /* ICH7 on Acer 3682WLMi */ + { 0x27DF, 0x1043, 0x1267 }, /* ICH7 on Asus W5F */ /* end marker */ { 0, } }; -- 1.5.0.5 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata-acpi: summary, problems, questions and proposal
Hello, Matthew. Matthew Garrett wrote: On Wed, Mar 28, 2007 at 04:30:02PM +0900, Tejun Heo wrote: Hi Tejun, Firstly, could I ask you to take a look at the patch in http://permalink.gmane.org/gmane.linux.acpi.devel/22066/ ? It deals with some of these issues. Yeap, I've seen the patch. That's why you're on the cc list in the first place. :-) ACPI support implementation in libata-dev supports both IDE and SATA ACPI object layouts and subset of ATA ACPI methods - _SDD and _GTF. It incorrectly uses ap-cbl (the port's cable type) to choose between the two ACPI layouts. Association between the host and its ACPI object is performed every time ACPI methods are invoked but the association between an ATA device and its ACPI object is cached in ata_device object. These issues are both fixed in my patch, I believe. Yeap, I think it's in the right direction but we need to go further. * I'm not sure whether the complex walk libata-acpi is doing is justifiable. * You'll end up doing _STM/_GTM on ahci controller on some BIOSen - 1. it can be dangerous 2. you might get partial or incorrect mapping - think about ICH8-split-to-two-PCI-fn-in-piix-mode case. 2-2. Missing proper _GTM/_STM support. As stated above, although -mm contains _GTM/_STM support, it does not hook it to regular exception handling path and thus _GTF cannot be used in a lot of cases. I've added _GTM and _STM support over suspend/resume. Right now they're in the host power management code - I'm not sure whether they should be here or the SCSI glue layer? I think PM functions in libata-eh is better place and you also need to do _GTF after _GTM during resume. 2-3. Misplaced _GTF hook. _GTF currently is called prior to every device configuration. This is unnecessary and incorrect. The ACPI spec specifies that _GTM/_STM and _GTF should be executed during suspend/resume cycles not on every reset or reconfiguration. This, for example, causes the following problem. That should be quite easily fixable with the above patch. Yeap. 4-1. Depending on how questions in section 3 are answered, fix and clean up ATA host/device - ACPI object association. Whether IDE or SATA native style hierarchy is used should be determined by driver flag not cable type. e.g. ahci and sata_sil24 should use SATA native style hierarchy while ata_piix should use IDE hierarchy whether the port is SATA or PATA. I think this is just a matter of making sure that the sata and pata handle matching code matches reality now :) Currently 1/2 of libata-acpi code is dealing with the above. I'm trying to figure out why it needs to be that complex. Anyways, I think your patch is a step in the right direction, so depending on how ACPI gurus enlighten us here, we can base further fix on your patch. Let's see how the questions are answered. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata-acpi: summary, problems, questions and proposal
On Thu, Mar 29, 2007 at 10:42:03AM +0900, Tejun Heo wrote: Yeap, I think it's in the right direction but we need to go further. * I'm not sure whether the complex walk libata-acpi is doing is justifiable. Yeah, that may well be less than ideal. * You'll end up doing _STM/_GTM on ahci controller on some BIOSen - 1. it can be dangerous 2. you might get partial or incorrect mapping - think about ICH8-split-to-two-PCI-fn-in-piix-mode case. So far I haven't seen any DSDTs that include _GTM and _STM methods for controllers that come up flagged as ahci, though I'm perfectly willing to believe that they're out there... I think PM functions in libata-eh is better place and you also need to do _GTF after _GTM during resume. Well, need - I haven't actually found hardware that seems upset about the missing _GTF call :) But you're right, it ought to be done. Can you point me at the right bits of the libata-eh code? I was trying to work through it earlier, but keeping track of the conditional paths is a bit tricky. I think this is just a matter of making sure that the sata and pata handle matching code matches reality now :) Currently 1/2 of libata-acpi code is dealing with the above. I'm trying to figure out why it needs to be that complex. I wrote a patch at one point that simplified this to an extent (http://lkml.org/lkml/2005/12/7/425), but it got somewhat bogged down in discussion about where the right place to do the binding was. Personally I'd prefer to handle this in a similar way to PCI - that is, register the ata bus with ACPI and then find handles as and when new ata devices are added to that bus. This has the advantage that it's easy to tie ACPI events to specific ata devices, which could then be integrated with the bay and dock drivers. -- Matthew Garrett | [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3] libata: check for AN support
Kristen Carlson Accardi wrote: Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. As supporting AN needs host interrupt handler change. I think we need host-supports-AN flag; otherwise, we might end up with screaming interrupts in the worst case. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/3] libata: check for AN support
Tejun Heo wrote: Kristen Carlson Accardi wrote: Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. As supporting AN needs host interrupt handler change. I think we need host-supports-AN flag; otherwise, we might end up with screaming interrupts in the worst case. Quite so. Lacking a host flag, we need to know how each and every controller behaves when AN is activated (and supported by the device). I'm willing to bet some of the first-gen SATA controllers' ASIC state machines croak when AN is activated. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/3] libata: expose AN support to user space via sysfs
Kristen Carlson Accardi wrote: Allow user space to determine if an ATAPI device supports async notification (AN) of media changes. This is done by adding a new sysfs file async_notification to genhd. If the file reads 1, then the device supports async notification. If the file reads 0, it does not. A flag is set in the generic disk to indicate whether or not AN is supported. This flag is set by the SCSI subsystem when it registers with add_disk. The SCSI system gets information from libata on whether the device supports AN during dev_configure. I'm not sure whether this should be in generic block layer or in libata proper. libata sysfs hierarchy isn't there yet but is scheduled to be added soon. Async notification of media change is generic event for any block device with removable media, so I guess it can belong to generic layer. BTW, I think you also need to forward the flag in sd - disk device can be removable too. And please cc linux-scsi@vger.kernel.org to get SCSI part reviewed. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/3] libata: expose AN support to user space via sysfs
Jeff Garzik wrote: Kristen Carlson Accardi wrote: Allow user space to determine if an ATAPI device supports async notification (AN) of media changes. This is done by adding a new sysfs file async_notification to genhd. If the file reads 1, then the device supports async notification. If the file reads 0, it does not. A flag is set in the generic disk to indicate whether or not AN is supported. This flag is set by the SCSI subsystem when it registers with add_disk. The SCSI system gets information from libata on whether the device supports AN during dev_configure. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] 3) I would make the contents of 'media_change_events' be a list of flags, rather than a boolean. Thus, when AN is present, media_change_events would return AN\n. It would return \n (no flags) when AN is absent. This permits future expansion of this capabilities reporting variable. I'm not sure about this. AN is kind of specific term for ATA while media change event is generic. So, I think the original approach is okay. No matter how the actual thing is implemented, it's the same media change event and as long as event delivery interface is the same, upper layer shouldn't care about how it's done. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] libata: handle AN interrupt
Kristen Carlson Accardi wrote: When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. The ahci host controller will send the event via KOBJ_CHANGE uevent. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] +static void async_notify_thread(struct work_struct *work) +{ + struct ata_device *atadev = + container_of(work, struct ata_device, async_notify); + + /* +* TBD - who should send this event? I couldn't find an +* easy way to map an ata_device to a genhd device, so +* decided maybe the ata host should send the event and +* allow user space to figure out what happened? +*/ + kobject_uevent(atadev-ap-host-dev-kobj, KOBJ_CHANGE); +} I don't think this is right. If you're gonna make media_change_event capability generic, you gotta make event delivery generic too. You can make it a genhd event and make genhd supply the interface function, say, genhd_notify_media_change() which is then forwarded by SCSI layer. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/3] libata: expose AN support to user space via sysfs
Tejun Heo wrote: Jeff Garzik wrote: Kristen Carlson Accardi wrote: Allow user space to determine if an ATAPI device supports async notification (AN) of media changes. This is done by adding a new sysfs file async_notification to genhd. If the file reads 1, then the device supports async notification. If the file reads 0, it does not. A flag is set in the generic disk to indicate whether or not AN is supported. This flag is set by the SCSI subsystem when it registers with add_disk. The SCSI system gets information from libata on whether the device supports AN during dev_configure. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] 3) I would make the contents of 'media_change_events' be a list of flags, rather than a boolean. Thus, when AN is present, media_change_events would return AN\n. It would return \n (no flags) when AN is absent. This permits future expansion of this capabilities reporting variable. I'm not sure about this. AN is kind of specific term for ATA while media change event is generic. So, I think the original approach is okay. No matter how the actual thing is implemented, it's the same media change event and as long as event delivery interface is the same, upper layer shouldn't care about how it's done. AN is a generic concept that I feel will propagate elsewhere. Though perhaps it should be in a 'capability_flags' file rather than a 'media_change_event' file. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata-acpi: summary, problems, questions and proposal
Matthew Garrett wrote: * You'll end up doing _STM/_GTM on ahci controller on some BIOSen - 1. it can be dangerous 2. you might get partial or incorrect mapping - think about ICH8-split-to-two-PCI-fn-in-piix-mode case. So far I haven't seen any DSDTs that include _GTM and _STM methods for controllers that come up flagged as ahci, though I'm perfectly willing to believe that they're out there... My ICH8 board seems to. I'll further look into it and post dsl files. I think PM functions in libata-eh is better place and you also need to do _GTF after _GTM during resume. Well, need - I haven't actually found hardware that seems upset about the missing _GTF call :) _GTF does the password unlocking and device configuration part of _STM. ACPI itself might not whine because no other ACPI method depends on _GTF being executed but functionally it's the most important piece. Now I think about it, _GTF is eventually done during device revalidation but that needs to be removed, so we'll need to add it in the resume path. But you're right, it ought to be done. Can you point me at the right bits of the libata-eh code? I was trying to work through it earlier, but keeping track of the conditional paths is a bit tricky. There is a big suspend/resume rewrite patch pending and the suspend/resume code will look quiet different (simpler) after it. I think the correct place for _STM/_GTF would be right after ata_eh_revalidate_and_attach(). As libata allows device hotplugging while suspended, we probably need to skip _STM/_GTF if the attached device isn't the one we've seen during suspending. Another problem is that _GTF might alter device size (number of blocks). Currently libata uses device size as one of the metrics to determine whether the device is the same one it saw last time during revalidation. This is pretty good safeguard as IIRC in some rare cases BIOS not only cuts sectors at the end of the disk but also offsets all the blocks (was this even standard? it can't be done using overlay or hpa. some vendor extension maybe?). So, if the different disk size is due to offset and you continue to operate on the disk as before, you're seriously screwed. We need to make sure that the device is the same one that we saw during suspend before doing _STM/_GTF and before _GTF the disk size might differ. Recent HPA handling Alan posted should be considered here too. Also, HPA programming might interact with _GTF. I think this is just a matter of making sure that the sata and pata handle matching code matches reality now :) Currently 1/2 of libata-acpi code is dealing with the above. I'm trying to figure out why it needs to be that complex. I wrote a patch at one point that simplified this to an extent (http://lkml.org/lkml/2005/12/7/425), but it got somewhat bogged down in discussion about where the right place to do the binding was. Personally I'd prefer to handle this in a similar way to PCI - that is, register the ata bus with ACPI and then find handles as and when new ata devices are added to that bus. This has the advantage that it's easy to tie ACPI events to specific ata devices, which could then be integrated with the bay and dock drivers. Yeap, that would be great. The problem is that libata isn't struct device'd (yet). As libata needs sysfs representation anyway, we might go all the way and implement ATA bus and all. Duplication between SCSI and ATA nodes is worrying but I guess we can figure out something. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions
On Wednesday 28 March 2007 18:38:48 Linus Torvalds wrote: On Wed, 28 Mar 2007, Maxim wrote: Now I don't have a clue how to set those bits if only HPET is used as clock source because now clocksources don't have _any_ resume hook. One thing that drives me wild about that clocksource resume thing is that it seems to think that clocksources are somehow different from any other system devices.. Why isn't the HPET considered a device, and has it's own *device* suspend and resume? Why do we seem to think that only set_mode() etc should wake up clock sources? It's a *device*, dammit. It should save and resume like one (probably as a system device). The set_mode() etc stuff is at a completely different (higher) conceptual level. Thomas? It does seem like Maxim has hit the nail on the head (at least partly) on the HPET timer resume problems.. Linus Hi, I am sending here a patch that as was discussed here adds hpet to list of system devices and adds suspend/resume hooks this way. I tested it and it works fine. --- Add suspend/resume support for HPET Signed-off-by: Maxim Levitsky [EMAIL PROTECTED] --- arch/i386/kernel/hpet.c | 64 +++ 1 files changed, 64 insertions(+), 0 deletions(-) diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c index 0fd9fba..ac41476 100644 --- a/arch/i386/kernel/hpet.c +++ b/arch/i386/kernel/hpet.c @@ -3,6 +3,8 @@ #include linux/errno.h #include linux/hpet.h #include linux/init.h +#include linux/sysdev.h +#include linux/pm.h #include asm/hpet.h #include asm/io.h @@ -524,3 +526,65 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } #endif + + +/* + * Suspend/resume part + */ + +#ifdef CONFIG_PM + +static int hpet_suspend(struct sys_device *sys_device, pm_message_t state) +{ + unsigned long cfg = hpet_readl(HPET_CFG); + + cfg = ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY); + hpet_writel(cfg, HPET_CFG); + + return 0; +} + +static int hpet_resume(struct sys_device *sys_device) +{ + unsigned int id; + + hpet_start_counter(); + + id = hpet_readl(HPET_ID); + + if (id HPET_ID_LEGSUP) + hpet_enable_int(); + + return 0; +} + +static struct sysdev_class hpet_class = { + set_kset_name(hpet), + .suspend= hpet_suspend, + .resume = hpet_resume, +}; + +static struct sys_device hpet_device = { + .id = 0, + .cls= hpet_class, +}; + + +static __init int hpet_register_sysfs(void) +{ + int err; + + err = sysdev_class_register(hpet_class); + + if (!err) { + sysdev_register(hpet_device); + if (err) + sysdev_class_unregister(hpet_class); + } + + return err; +} + +device_initcall(hpet_register_sysfs); + +#endif -- 1.4.4.2 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions
On Thu, 29 Mar 2007, Maxim wrote: I am sending here a patch that as was discussed here adds hpet to list of system devices and adds suspend/resume hooks this way. I tested it and it works fine. Ok, it certainly looks better, but it *also* looks like it just assumes the HPET is there. Which would work in testing _with_ a HPET, but would likely break on hardware without one, no? Shouldn't there be at least something like a if (!is_hpet_capable()) return 0; at the top of that init routine? I'd also expect that you'd need to check that hpet_virt_address is valid or something? (Or, better yet, shouldn't we set boot_hpet_disable when we decide not to use the HPET, and set hpet_virt_address to NULL?) Linus - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ PATCH] Add suspend/resume for HPET was: Re: [3/6] 2.6.21-rc4: known regressions
On Thursday 29 March 2007 07:08:58 Linus Torvalds wrote: On Thu, 29 Mar 2007, Maxim wrote: I am sending here a patch that as was discussed here adds hpet to list of system devices and adds suspend/resume hooks this way. I tested it and it works fine. Ok, it certainly looks better, but it *also* looks like it just assumes the HPET is there. Which would work in testing _with_ a HPET, but would likely break on hardware without one, no? Shouldn't there be at least something like a if (!is_hpet_capable()) return 0; at the top of that init routine? I'd also expect that you'd need to check that hpet_virt_address is valid or something? (Or, better yet, shouldn't we set boot_hpet_disable when we decide not to use the HPET, and set hpet_virt_address to NULL?) This is done here out_nohpet: iounmap(hpet_virt_address); hpet_virt_address = NULL; Linus Hi, Of course, I forgot. I was planning to put sysdev code in hpet_enable() but it is not possible because this function is called too early. Thus I put sysdev initialization in separate function but forgot to test for HPET Thanks a lot. Best regards Maxim Levitsky --- This adds support of suspend/resume on i386 for HPET Signed-off-by: Maxim Levitsky [EMAIL PROTECTED] --- arch/i386/kernel/hpet.c | 68 +++ 1 files changed, 68 insertions(+), 0 deletions(-) diff --git a/arch/i386/kernel/hpet.c b/arch/i386/kernel/hpet.c index 0fd9fba..7c67780 100644 --- a/arch/i386/kernel/hpet.c +++ b/arch/i386/kernel/hpet.c @@ -3,6 +3,8 @@ #include linux/errno.h #include linux/hpet.h #include linux/init.h +#include linux/sysdev.h +#include linux/pm.h #include asm/hpet.h #include asm/io.h @@ -310,6 +312,7 @@ int __init hpet_enable(void) out_nohpet: iounmap(hpet_virt_address); hpet_virt_address = NULL; + boot_hpet_disable = 1; return 0; } @@ -524,3 +527,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } #endif + + +/* + * Suspend/resume part + */ + +#ifdef CONFIG_PM + +static int hpet_suspend(struct sys_device *sys_device, pm_message_t state) +{ + unsigned long cfg = hpet_readl(HPET_CFG); + + cfg = ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY); + hpet_writel(cfg, HPET_CFG); + + return 0; +} + +static int hpet_resume(struct sys_device *sys_device) +{ + unsigned int id; + + hpet_start_counter(); + + id = hpet_readl(HPET_ID); + + if (id HPET_ID_LEGSUP) + hpet_enable_int(); + + return 0; +} + +static struct sysdev_class hpet_class = { + set_kset_name(hpet), + .suspend= hpet_suspend, + .resume = hpet_resume, +}; + +static struct sys_device hpet_device = { + .id = 0, + .cls= hpet_class, +}; + + +static __init int hpet_register_sysfs(void) +{ + int err; + + if (!is_hpet_capable()) + return 0; + + err = sysdev_class_register(hpet_class); + + if (!err) { + sysdev_register(hpet_device); + if (err) + sysdev_class_unregister(hpet_class); + } + + return err; +} + +device_initcall(hpet_register_sysfs); + +#endif -- 1.4.4.2 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html