libata-acpi: summary, problems, questions and proposal

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Jeff Garzik

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

2007-03-28 Thread Jeff Garzik

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

2007-03-28 Thread Thomas Gleixner
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

2007-03-28 Thread Alan Cox
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)

2007-03-28 Thread Pavel Machek
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)

2007-03-28 Thread Tejun Heo
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

2007-03-28 Thread Rodney Padgett
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

2007-03-28 Thread Maxim
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)

2007-03-28 Thread Sergei Shtylyov
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

2007-03-28 Thread Matthew Garrett
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)

2007-03-28 Thread Sergei Shtylyov

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)

2007-03-28 Thread Sergei Shtylyov
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

2007-03-28 Thread Sergei Shtylyov
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

2007-03-28 Thread Geoff Rivell
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)

2007-03-28 Thread Jeff Garzik

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)

2007-03-28 Thread Phillip Susi

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

2007-03-28 Thread Laurent Riffard

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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Alan Cox
 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

2007-03-28 Thread Linus Torvalds


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

2007-03-28 Thread Andrew Morton
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

2007-03-28 Thread Kyle McMartin
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

2007-03-28 Thread Kristen Carlson Accardi
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

2007-03-28 Thread Kristen Carlson Accardi
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

2007-03-28 Thread Kristen Carlson Accardi
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

2007-03-28 Thread Kristen Carlson Accardi
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

2007-03-28 Thread Jeff Garzik

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

2007-03-28 Thread Ingo Molnar

* 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

2007-03-28 Thread Linus Torvalds


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

2007-03-28 Thread Michael S. Tsirkin
 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

2007-03-28 Thread Ingo Molnar

* 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

2007-03-28 Thread Randy Dunlap
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

2007-03-28 Thread David Brownell
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

2007-03-28 Thread Maxim
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

2007-03-28 Thread Linus Torvalds


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

2007-03-28 Thread David Brownell
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

2007-03-28 Thread David Brownell
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

2007-03-28 Thread Maxim
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

2007-03-28 Thread Maxim
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.

2007-03-28 Thread Robin H\. Johnson
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

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Matthew Garrett
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

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Jeff Garzik

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

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Tejun Heo

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

2007-03-28 Thread Jeff Garzik

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

2007-03-28 Thread Tejun Heo
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

2007-03-28 Thread Maxim
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

2007-03-28 Thread Linus Torvalds


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

2007-03-28 Thread Maxim
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