libata: improve ATA ACPI support
PATCH 1/1 libata: improve ATA ACPI support
Hello, all.
This patchset improves ATA ACPI support as proposed in the following message.
http://article.gmane.org/gmane.linux.ide/17554
Improvements are...
* safe and simpler ACPI node association
* major code cleanup
* invoke ACPI methods only when ACPI spec says necessary
* proper ACPI error handling with a retry
* after successfully executing _GTF taskfiles, IDENTIFY page is reloaded
* _GTM/_STM support
This patchset is composed of 13 patches.
#01-02 : misc preparation
#03-04 : separate out ata_dev_reread_id() and make revalidation robust
against size change during device configuration
#05-06 : clean up libata-acpi.c
#07-08 : implement ata_acpi_associate()
#09-10 : more cleanups
#11-13 : reimplement ACPI invocation so that methods are called where
the spec specifies and errors are properly handled, and add _GTM/_STM support.
Tested on three desktop boards (nf ultra, ich7 and ich8) and a notebook. This patchset is against...
upstream (5365067b4bb17d1801fefe995d1342108b324471)
+ [1] pata_amd-remove-contamination
+ [2] libata-add-missing-call-to-cable_detect-in-new-EH
+ [3] libata-acpi-fix-GTF-command-protocol-for-ATAPI
Thanks.
--
tejun
[1] http://article.gmane.org/gmane.linux.ide/18120
[2] http://article.gmane.org/gmane.linux.ide/18121
[3] http://article.gmane.org/gmane.linux.ide/18122
PATCH 1/13 ahci: consolidate common port flags
Consolidate common port flags into AHCI_FLAG_COMMON.
--- drivers/ata/ahci.c | 29 ++++++++++------------------- 1 files changed, 10 insertions(+), 19 deletions(-)
drivers/ata/ahci.c +10 -19
@@ -170,6 +170,10 @@ enum {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 */ + +AHCI_FLAG_COMMON =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | +ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | + ATA_FLAG_SKIP_D2H_BSY, }; struct ahci_cmd_hdr {
@@ -323,54 +327,41 @@ static const struct ata_port_operations ahci_vt8251_ops = {static const struct ata_port_info ahci_port_info[] = {/* board_ahci */ { - .flags =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | -ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -ATA_FLAG_SKIP_D2H_BSY , + .flags =AHCI_FLAG_COMMON , .pio_mask =0x1f ,/* pio0-4 */ .udma_mask =0x7f ,/* udma0-6 ; FIXME */ .port_ops = &ahci_ops, },/* board_ahci_pi */ { - .flags =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | -ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -ATA_FLAG_SKIP_D2H_BSY |AHCI_FLAG_HONOR_PI , + .flags =AHCI_FLAG_COMMON |AHCI_FLAG_HONOR_PI , .pio_mask =0x1f ,/* pio0-4 */ .udma_mask =0x7f ,/* udma0-6 ; FIXME */ .port_ops = &ahci_ops, },/* board_ahci_vt8251 */ { - .flags =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | -ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -ATA_FLAG_SKIP_D2H_BSY | -ATA_FLAG_HRST_TO_RESUME |AHCI_FLAG_NO_NCQ , + .flags =AHCI_FLAG_COMMON |ATA_FLAG_HRST_TO_RESUME | +AHCI_FLAG_NO_NCQ , .pio_mask =0x1f ,/* pio0-4 */ .udma_mask =0x7f ,/* udma0-6 ; FIXME */ .port_ops = &ahci_vt8251_ops, },/* board_ahci_ign_iferr */ { - .flags =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | -ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -ATA_FLAG_SKIP_D2H_BSY | -AHCI_FLAG_IGN_IRQ_IF_ERR , + .flags =AHCI_FLAG_COMMON |AHCI_FLAG_IGN_IRQ_IF_ERR , .pio_mask =0x1f ,/* pio0-4 */ .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 | + .flags =AHCI_FLAG_COMMON |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[] = {
Signature · 1 line
1.5.0.3
PATCH 2/13 libata: separate ATA_EHI_DID_RESET into DID_SOFTRESET and DID_HARDRESET
In reply to Tejun HeoSeparate ATA_EHI_DID_RESET into ATA_EHI_DID_SOFTRESET and ATA_EHI_DID_HARDRESET. ATA_EHI_DID_RESET is redefined as OR of the two flags. This patch doesn't introduce any behavior change. This will be used later to determine whether _SDD is necessary or not.
--- drivers/ata/libata-eh.c | 5 ++++- include/linux/libata.h | 10 ++++++---- 2 files changed, 10 insertions(+), 5 deletions(-)
drivers/ata/libata-eh.c +4 -1
@@ -1671,7 +1671,10 @@ static int ata_eh_reset(struct ata_port *ap, int classify, reset == softreset ? "soft" : "hard");/* mark that this EH session started with reset */ - ehc->i.flags |=ATA_EHI_DID_RESET ; +if (reset == hardreset) + ehc->i.flags |=ATA_EHI_DID_HARDRESET ; +else + ehc->i.flags |=ATA_EHI_DID_SOFTRESET ; rc =ata_do_reset (ap, reset, classes);
include/linux/libata.h +6 -4
@@ -282,11 +282,13 @@ enum {ATA_EHI_NO_AUTOPSY = (1 <<2 ),/* no autopsy */ ATA_EHI_QUIET = (1 <<3 ),/* be quiet */ -ATA_EHI_DID_RESET = (1 <<16 ),/* already reset this port */ -ATA_EHI_PRINTINFO = (1 <<17 ),/* print configuration info */ -ATA_EHI_SETMODE = (1 <<18 ),/* configure transfer mode */ -ATA_EHI_POST_SETMODE = (1 <<19 ),/* revaildating after setmode */ +ATA_EHI_DID_SOFTRESET = (1 <<16 ),/* already soft-reset this port */ +ATA_EHI_DID_HARDRESET = (1 <<17 ),/* already soft-reset this port */ +ATA_EHI_PRINTINFO = (1 <<18 ),/* print configuration info */ +ATA_EHI_SETMODE = (1 <<19 ),/* configure transfer mode */ +ATA_EHI_POST_SETMODE = (1 <<20 ),/* revaildating after setmode */ +ATA_EHI_DID_RESET =ATA_EHI_DID_SOFTRESET |ATA_EHI_DID_HARDRESET ,ATA_EHI_RESET_MODIFIER_MASK =ATA_EHI_RESUME_LINK , /* max repeat if error condition is still set after ->error_handler */
Signature · 1 line
1.5.0.3
PATCH 3/13 libata: separate out ata_dev_reread_id()
In reply to Tejun HeoSeparate out ata_dev_reread_id() from ata_dev_revalidate(). ata_dev_reread_id() reads IDENTIFY page and determines whether the same device is still there. ata_dev_revalidate() reconfigures after reread completes. This will be used by ACPI update.
--- drivers/ata/libata-core.c | 47 ++++++++++++++++++++++++++++++++------------ drivers/ata/libata.h | 3 +- 2 files changed, 36 insertions(+), 14 deletions(-)
drivers/ata/libata-core.c +34 -13
@@ -3562,8 +3562,8 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class, }/** -* ata_dev_revalidate - Revalidate ATA device -* @dev: device to revalidate +* ata_dev_reread_id - Re-read IDENTIFY data +* @adev: target ATA device * @readid_flags: read ID flags * * Re-read IDENTIFY page and make sure @dev is still attached to
@@ -3575,29 +3575,50 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,* RETURNS: * 0 on success, negative errno otherwise */ -int ata_dev_revalidate (struct ata_device *dev,unsigned int readid_flags) +int ata_dev_reread_id (struct ata_device *dev,unsigned int readid_flags) {unsigned int class = dev->class;u16 *id = (void *)dev->ap->sector_buf;int rc; -if (!ata_dev_enabled (dev)) { - rc = -ENODEV ; - goto fail; - } -/* read ID data */ rc =ata_dev_read_id (dev, &class, readid_flags, id);if (rc) - goto fail; +return rc;/* is the device still there? */ -if (!ata_dev_same_device (dev, class, id)) { - rc = -ENODEV ; - goto fail; - } +if (!ata_dev_same_device (dev, class, id)) +return -ENODEV ;memcpy (dev->id, id,sizeof (id[0 ]) *ATA_ID_WORDS ); +return 0 ; +} + +/** +* ata_dev_revalidate - Revalidate ATA device +* @dev: device to revalidate +* @readid_flags: read ID flags +* +* Re-read IDENTIFY page, make sure @dev is still attached to the +* port and reconfigure it according to the new IDENTIFY page. +* +* LOCKING: +* Kernel thread context (may sleep) +* +* RETURNS: +* 0 on success, negative errno otherwise +*/ +int ata_dev_revalidate (struct ata_device *dev,unsigned int readid_flags) +{ +int rc; + +if (!ata_dev_enabled (dev)) +return -ENODEV ; + +/* re-read ID */ + rc =ata_dev_reread_id (dev, readid_flags); +if (rc) + goto fail;/* configure device according to the new ID */ rc =ata_dev_configure (dev);
drivers/ata/libata.h +2 -1
@@ -75,7 +75,8 @@ extern unsigned ata_exec_internal_sg(struct ata_device *dev,extern unsigned int ata_do_simple_cmd (struct ata_device *dev,u8 cmd);extern int ata_dev_read_id (struct ata_device *dev,unsigned int *p_class,unsigned int flags,u16 *id); -extern int ata_dev_revalidate (struct ata_device *dev,unsigned int flags); +extern int ata_dev_reread_id (struct ata_device *dev,unsigned int readid_flags); +extern int ata_dev_revalidate (struct ata_device *dev,unsigned int readid_flags);extern int ata_dev_configure (struct ata_device *dev);extern int sata_down_spd_limit (struct ata_port *ap);extern int sata_set_spd_needed (struct ata_port *ap);
Signature · 1 line
1.5.0.3
PATCH 4/13 libata: during revalidation, check n_sectors after device is configured
In reply to Tejun HeoDevice might be resized during ata_dev_configure() due to HPA or (later) ACPI _GTF. Currently it's worked around by caching n_sectors before turning off HPA. The cached original size is overwritten if the device is reconfigured without being hardreset - which always happens after configuring trasnfer mode. If the device gets hardreset for some reason after that, revalidation fails with -ENODEV.
This patch makes size checking more robust by moving n_sectors check from ata_dev_reread_id() to ata_dev_revalidate() after the device is fully configured. No matter what happens during configuration, a device must have the same n_sectors after fully configured to be treated as the same device.
--- drivers/ata/libata-core.c | 33 +++++++++++++++------------------ include/linux/libata.h | 1 - 2 files changed, 15 insertions(+), 19 deletions(-)
drivers/ata/libata-core.c +15 -18
@@ -1862,7 +1862,6 @@ int ata_dev_configure(struct ata_device *dev)snprintf (revbuf,7 ,"ATA-%d" ,ata_id_major_version (id)); dev->n_sectors =ata_id_n_sectors (id); - dev->n_sectors_boot = dev->n_sectors;/* SCSI only uses 4-char revisions, dump full 8 chars from ATA */ ata_id_c_string(dev->id, fwrevbuf, ATA_ID_FW_REV,
@@ -3519,7 +3518,6 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,const u16 *old_id = dev->id;unsigned char model[2 ][ATA_ID_PROD_LEN +1 ];unsigned char serial[2 ][ATA_ID_SERNO_LEN +1 ]; -u64 new_n_sectors;if (dev->class != new_class) { ata_dev_printk(dev, KERN_INFO, "class mismatch %d != %d\n",
@@ -3531,7 +3529,6 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,ata_id_c_string (new_id, model[1 ],ATA_ID_PROD ,sizeof (model[1 ]));ata_id_c_string (old_id, serial[0 ],ATA_ID_SERNO ,sizeof (serial[0 ]));ata_id_c_string (new_id, serial[1 ],ATA_ID_SERNO ,sizeof (serial[1 ])); - new_n_sectors =ata_id_n_sectors (new_id);if (strcmp (model[0 ], model[1 ])) { ata_dev_printk(dev, KERN_INFO, "model number mismatch "
@@ -3545,19 +3542,6 @@ static int ata_dev_same_device(struct ata_device *dev, unsigned int new_class,return 0 ; } -if (dev->class ==ATA_DEV_ATA && dev->n_sectors != new_n_sectors) { -ata_dev_printk (dev,KERN_INFO ,"n_sectors mismatch " -"%llu != %llu\n" , - (unsigned long long )dev->n_sectors, - (unsigned long long )new_n_sectors); -/* Are we the boot time size - if so we appear to be the -same disk at this point and our HPA got reapplied */ -if (ata_ignore_hpa && dev->n_sectors_boot == new_n_sectors - &&ata_id_hpa_enabled (new_id)) -return 1 ; -return 0 ; - } -return 1 ; }
@@ -3610,6 +3594,7 @@ int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags)*/ int ata_dev_revalidate (struct ata_device *dev,unsigned int readid_flags) { +u64 n_sectors = dev->n_sectors;int rc;if (!ata_dev_enabled (dev))
@@ -3622,8 +3607,20 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int readid_flags)/* configure device according to the new ID */ rc =ata_dev_configure (dev); -if (rc ==0 ) -return 0 ; +if (rc) + goto fail; + +/* verify n_sectors hasn't changed */ +if (dev->class ==ATA_DEV_ATA && dev->n_sectors != n_sectors) { +ata_dev_printk (dev,KERN_INFO ,"n_sectors mismatch " +"%llu != %llu\n" , + (unsigned long long )n_sectors, + (unsigned long long )dev->n_sectors); + rc = -ENODEV ; + goto fail; + } + +return 0 ; fail:ata_dev_printk (dev,KERN_ERR ,"revalidation failed (errno=%d)\n" , rc);
include/linux/libata.h +0 -1
@@ -448,7 +448,6 @@ struct ata_device {struct scsi_device *sdev;/* attached SCSI device */ /* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */ u64 n_sectors;/* size of device, if ATA */ -u64 n_sectors_boot;/* size of ATA device at startup */ unsigned int class;/* ATA_DEV_xxx */ u16 id[ATA_ID_WORDS ];/* IDENTIFY xxx DEVICE data */ u8 pio_mode;
Signature · 1 line
1.5.0.3
PATCH 5/13 libata-acpi: s/CONFIG_SATA_ACPI/CONFIG_ATA_ACPI/
In reply to Tejun HeoACPI applies to both SATA and PATA. Drop the 'S' from the config variable.
--- drivers/ata/Kconfig | 26 +++++++++++++------------- drivers/ata/Makefile | 2 +- drivers/ata/libata.h | 2 +- include/linux/libata.h | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-)
drivers/ata/Kconfig +13 -13
@@ -23,6 +23,19 @@ config ATA_NONSTANDARD bool default n +config ATA_ACPI + bool + depends on ACPI && PCI + default y + help + This option adds support for ATA-related ACPI objects. + These ACPI objects add the ability to retrieve taskfiles + from the ACPI BIOS and write them to the disk controller. + These objects may be related to performance, security, + power management, or other areas. + You can disable this at kernel boot time by using the + option libata.noacpi=1 + config SATA_AHCI tristate "AHCI SATA support" depends on PCI
@@ -156,19 +169,6 @@ config SATA_INIC162X help This option enables support for Initio 162x Serial ATA. -config SATA_ACPI - bool - depends on ACPI && PCI - default y - help - This option adds support for SATA-related ACPI objects. - These ACPI objects add the ability to retrieve taskfiles - from the ACPI BIOS and write them to the disk controller. - These objects may be related to performance, security, - power management, or other areas. - You can disable this at kernel boot time by using the - option libata.noacpi=1 - config PATA_ALI tristate "ALi PATA support (Experimental)" depends on PCI && EXPERIMENTAL
drivers/ata/Makefile +1 -1
@@ -68,4 +68,4 @@ obj-$(CONFIG_ATA_GENERIC) += ata_generic.o obj-$(CONFIG_PATA_LEGACY) += pata_legacy.o libata-objs := libata-core.o libata-scsi.o libata-sff.o libata-eh.o -libata-$(CONFIG_SATA_ACPI) += libata-acpi.o +libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
drivers/ata/libata.h +1 -1
@@ -97,7 +97,7 @@ extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);extern struct ata_port *ata_port_alloc (struct ata_host *host);/* libata-acpi.c */ -#ifdef CONFIG_SATA_ACPI +#ifdef CONFIG_ATA_ACPI extern int ata_acpi_exec_tfs (struct ata_port *ap);extern int ata_acpi_push_id (struct ata_port *ap,unsigned int ix);#else
include/linux/libata.h +1 -1
@@ -474,7 +474,7 @@ struct ata_device {struct ata_ering ering;int spdn_cnt;unsigned int horkage;/* List of broken features */ -#ifdef CONFIG_SATA_ACPI +#ifdef CONFIG_ATA_ACPI /* ACPI objects info */ acpi_handle obj_handle;#endif
Signature · 1 line
1.5.0.3
PATCH 6/13 libata-acpi: clean up parameters and misc stuff
In reply to Tejun HeoThis patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes.
* make libata-acpi functions take ata_device instead of ata_port + device index
* s/atadev/adev/
* de-indent local variable declarations
--- drivers/ata/libata-acpi.c | 184 ++++++++++++++++++++++----------------------- drivers/ata/libata-core.c | 2 +- drivers/ata/libata.h | 4 +- 3 files changed, 94 insertions(+), 96 deletions(-)
drivers/ata/libata-acpi.c +91 -93
@@ -270,8 +270,7 @@ out:/** * do_drive_get_GTF - get the drive bootup default taskfile settings -* @ap: the ata_port for the drive -* @ix: target ata_device (drive) index +* @adev: target ATA device * @gtf_length: number of bytes of _GTF data returned at @gtf_address * @gtf_address: buffer containing _GTF taskfile arrays *
@@ -286,20 +285,19 @@ out:* The returned @gtf_length and @gtf_address are only valid if the * function return value is 0. */ -static int do_drive_get_GTF (struct ata_port *ap,int ix, -unsigned int *gtf_length,unsigned long *gtf_address, -unsigned long *obj_loc) +static int do_drive_get_GTF (struct ata_device *adev,unsigned int *gtf_length, +unsigned long *gtf_address,unsigned long *obj_loc) { -acpi_status status; -acpi_handle dev_handle =NULL ; -acpi_handle chan_handle, drive_handle; -acpi_integer pcidevfn =0 ; -u32 dev_adr; -struct acpi_buffer output; -union acpi_object *out_obj; -struct device *dev = ap->host->dev; -struct ata_device *atadev = &ap->device[ix]; -int err = -ENODEV ; +struct ata_port *ap = adev->ap; +acpi_status status; +acpi_handle dev_handle =NULL ; +acpi_handle chan_handle, drive_handle; +acpi_integer pcidevfn =0 ; +u32 dev_adr; +struct acpi_buffer output; +union acpi_object *out_obj; +struct device *dev = ap->host->dev; +int err = -ENODEV ; *gtf_length =0 ; *gtf_address =0UL ;
@@ -309,14 +307,14 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,return 0 ;if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , +ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , __FUNCTION__, ap->port_no); -if (!ata_dev_enabled (atadev) || (ap->flags &ATA_FLAG_DISABLED )) { +if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED )) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: ERR: " +ata_dev_printk (adev,KERN_DEBUG ,"%s: ERR: " "ata_dev_present: %d, PORT_DISABLED: %lu\n" , - __FUNCTION__,ata_dev_enabled (atadev), + __FUNCTION__,ata_dev_enabled (adev), ap->flags &ATA_FLAG_DISABLED ); goto out; }
@@ -327,7 +325,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, err =pata_get_dev_handle (dev, &dev_handle, &pcidevfn);if (err <0 ) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: pata_get_dev_handle failed (%d)\n" , __FUNCTION__, err); goto out;
@@ -336,7 +334,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, err =sata_get_dev_handle (dev, &dev_handle, &pcidevfn);if (err <0 ) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: sata_get_dev_handle failed (%d\n" , __FUNCTION__, err); goto out;
@@ -344,7 +342,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, }/* Get this drive's _ADR info. if not already known. */ -if (!atadev->obj_handle) { +if (!adev->obj_handle) {if (!(ap->cbl ==ATA_CBL_SATA )) {/* get child objects of dev_handle == channel objects, * + _their_ children == drive objects */
@@ -352,7 +350,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, chan_handle =acpi_get_child (dev_handle, ap->port_no);if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: chan adr=%d: chan_handle=0x%p\n" , __FUNCTION__, ap->port_no, chan_handle);
@@ -361,26 +359,26 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, goto out; }/* TBD: could also check ACPI object VALID bits */ - drive_handle =acpi_get_child (chan_handle, ix); + drive_handle =acpi_get_child (chan_handle, adev->devno);if (!drive_handle) { err = -ENODEV ; goto out; } - dev_adr = ix; - atadev->obj_handle = drive_handle; + dev_adr = adev->devno; + adev->obj_handle = drive_handle; }else {/* for SATA mode */ dev_adr =SATA_ADR_RSVD ; err =get_sata_adr (dev, dev_handle, pcidevfn,0 , - ap, atadev, &dev_adr); + ap, adev, &dev_adr); }if (err <0 || dev_adr ==SATA_ADR_RSVD || - !atadev->obj_handle) { + !adev->obj_handle) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: get_sata/pata_adr failed: " "err=%d, dev_adr=%u, obj_handle=0x%p\n" , __FUNCTION__, err, dev_adr, - atadev->obj_handle); + adev->obj_handle); goto out; } }
@@ -391,11 +389,11 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,/* _GTF has no input parameters */ err = -EIO ; - status =acpi_evaluate_object (atadev->obj_handle,"_GTF" , + status =acpi_evaluate_object (adev->obj_handle,"_GTF" ,NULL , &output);if (ACPI_FAILURE (status)) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: Run _GTF error: status = 0x%x\n" , __FUNCTION__, status); goto out;
@@ -403,7 +401,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,if (!output.length || !output.pointer) {if (ata_msg_probe (ap)) - ata_dev_printk(atadev, KERN_DEBUG, "%s: Run _GTF: " + ata_dev_printk(adev, KERN_DEBUG, "%s: Run _GTF: " "length or ptr is NULL (0x%llx, 0x%p)\n", __FUNCTION__, (unsigned long long)output.length,
@@ -416,7 +414,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,if (out_obj->type !=ACPI_TYPE_BUFFER ) {kfree (output.pointer);if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: Run _GTF: " +ata_dev_printk (adev,KERN_DEBUG ,"%s: Run _GTF: " "error: expected object type of " " ACPI_TYPE_BUFFER, got 0x%x\n" , __FUNCTION__, out_obj->type);
@@ -427,7 +425,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,if (!out_obj->buffer.length || !out_obj->buffer.pointer || out_obj->buffer.length %REGS_PER_GTF ) {if (ata_msg_drv (ap)) -ata_dev_printk (atadev,KERN_ERR , +ata_dev_printk (adev,KERN_ERR ,"%s: unexpected GTF length (%d) or addr (0x%p)\n" , __FUNCTION__, out_obj->buffer.length, out_obj->buffer.pointer);
@@ -439,7 +437,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix, *gtf_address = (unsigned long )out_obj->buffer.pointer; *obj_loc = (unsigned long )out_obj;if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: returning " +ata_dev_printk (adev,KERN_DEBUG ,"%s: returning " "gtf_length=%d, gtf_address=0x%lx, obj_loc=0x%lx\n" , __FUNCTION__, *gtf_length, *gtf_address, *obj_loc); err =0 ;
@@ -449,7 +447,7 @@ out:/** * taskfile_load_raw - send taskfile registers to host controller -* @ap: Port to which output is sent +* @adev: target ATA device * @gtf: raw ATA taskfile register set (0x1f1 - 0x1f7) * * Outputs ATA taskfile to standard ATA host controller using MMIO
@@ -466,15 +464,15 @@ out:* LOCKING: TBD: * Inherited from caller. */ -static void taskfile_load_raw (struct ata_port *ap, -struct ata_device *atadev, -const struct taskfile_array *gtf) +static void taskfile_load_raw (struct ata_device *adev, +const struct taskfile_array *gtf) { +struct ata_port *ap = adev->ap;struct ata_taskfile tf;unsigned int err;if (ata_msg_probe (ap)) - ata_dev_printk(atadev, KERN_DEBUG, "%s: (0x1f1-1f7): hex: " + ata_dev_printk(adev, KERN_DEBUG, "%s: (0x1f1-1f7): hex: " "%02x %02x %02x %02x %02x %02x %02x\n", __FUNCTION__, gtf->tfa[0], gtf->tfa[1], gtf->tfa[2],
@@ -485,7 +483,7 @@ static void taskfile_load_raw(struct ata_port *ap, && (gtf->tfa[6] == 0))return ; -ata_tf_init (atadev, &tf); +ata_tf_init (adev, &tf);/* convert gtf to tf */ tf.flags |=ATA_TFLAG_ISADDR |ATA_TFLAG_DEVICE ;/* TBD */
@@ -498,17 +496,16 @@ static void taskfile_load_raw(struct ata_port *ap, tf.device = gtf->tfa[5 ];/* 0x1f6 */ tf.command = gtf->tfa[6 ];/* 0x1f7 */ - err =ata_exec_internal (atadev, &tf,NULL ,DMA_NONE ,NULL ,0 ); + err =ata_exec_internal (adev, &tf,NULL ,DMA_NONE ,NULL ,0 );if (err &&ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_ERR , +ata_dev_printk (adev,KERN_ERR ,"%s: ata_exec_internal failed: %u\n" , __FUNCTION__, err); }/** * do_drive_set_taskfiles - write the drive taskfile settings from _GTF -* @ap: the ata_port for the drive -* @atadev: target ata_device +* @adev: target ATA device * @gtf_length: total number of bytes of _GTF taskfiles * @gtf_address: location of _GTF taskfile arrays *
@@ -517,30 +514,31 @@ static void taskfile_load_raw(struct ata_port *ap,* Write {gtf_address, length gtf_length} in groups of * REGS_PER_GTF bytes. */ -static int do_drive_set_taskfiles (struct ata_port *ap, -struct ata_device *atadev,unsigned int gtf_length, -unsigned long gtf_address) +static int do_drive_set_taskfiles (struct ata_device *adev, +unsigned int gtf_length, +unsigned long gtf_address) { -int err = -ENODEV ; -int gtf_count = gtf_length /REGS_PER_GTF ; -int ix; +struct ata_port *ap = adev->ap; +int err = -ENODEV ; +int gtf_count = gtf_length /REGS_PER_GTF ; +int ix;struct taskfile_array *gtf;if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , +ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , __FUNCTION__, ap->port_no);if (libata_noacpi || !(ap->cbl ==ATA_CBL_SATA ))return 0 ; -if (!ata_dev_enabled (atadev) || (ap->flags &ATA_FLAG_DISABLED )) +if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED )) goto out;if (!gtf_count)/* shouldn't be here */ goto out;if (gtf_length %REGS_PER_GTF ) {if (ata_msg_drv (ap)) -ata_dev_printk (atadev,KERN_ERR , +ata_dev_printk (adev,KERN_ERR ,"%s: unexpected GTF length (%d)\n" , __FUNCTION__, gtf_length); goto out;
@@ -551,7 +549,7 @@ static int do_drive_set_taskfiles(struct ata_port *ap, (gtf_address + ix *REGS_PER_GTF );/* send all TaskFile registers (0x1f1-0x1f7) *in*that*order* */ -taskfile_load_raw (ap, atadev, gtf); +taskfile_load_raw (adev, gtf); } err =0 ;
@@ -567,11 +565,11 @@ out:*/ int ata_acpi_exec_tfs (struct ata_port *ap) { -int ix; -int ret =0 ; -unsigned int gtf_length; -unsigned long gtf_address; -unsigned long obj_loc; +int ix; +int ret =0 ; +unsigned int gtf_length; +unsigned long gtf_address; +unsigned long obj_loc;if (libata_noacpi)return 0 ;
@@ -584,11 +582,13 @@ int ata_acpi_exec_tfs(struct ata_port *ap)return 0 ;for (ix =0 ; ix <ATA_MAX_DEVICES ; ix++) { -if (!ata_dev_enabled (&ap->device[ix])) +struct ata_device *adev = &ap->device[ix]; + +if (!ata_dev_enabled (adev))continue ; - ret =do_drive_get_GTF (ap, ix, - >f_length, >f_address, &obj_loc); + ret =do_drive_get_GTF (adev, >f_length, >f_address, + &obj_loc);if (ret <0 ) {if (ata_msg_probe (ap)) ata_port_printk(ap, KERN_DEBUG,
@@ -597,8 +597,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)break ; } - ret =do_drive_set_taskfiles (ap, &ap->device[ix], - gtf_length, gtf_address); + ret =do_drive_set_taskfiles (adev, gtf_length, gtf_address);kfree ((void *)obj_loc);if (ret <0 ) {if (ata_msg_probe (ap))
@@ -614,8 +613,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)/** * ata_acpi_push_id - send Identify data to drive -* @ap: the ata_port for the drive -* @ix: drive index +* @adev: target ATA device * * _SDD ACPI object: for SATA mode only * Must be after Identify (Packet) Device -- uses its data
@@ -623,29 +621,29 @@ int ata_acpi_exec_tfs(struct ata_port *ap)* method and if it fails for whatever reason, we should still * just keep going. */ -int ata_acpi_push_id (struct ata_port *ap,unsigned int ix) +int ata_acpi_push_id (struct ata_device *adev) { -acpi_handle handle; -acpi_integer pcidevfn; -int err; -struct device *dev = ap->host->dev; -struct ata_device *atadev = &ap->device[ix]; -u32 dev_adr; -acpi_status status; -struct acpi_object_list input; -union acpi_object in_params[1 ]; +struct ata_port *ap = adev->ap; +acpi_handle handle; +acpi_integer pcidevfn; +int err; +struct device *dev = ap->host->dev; +u32 dev_adr; +acpi_status status; +struct acpi_object_list input; +union acpi_object in_params[1 ];if (libata_noacpi)return 0 ;if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG ,"%s: ix = %d, port#: %d\n" , - __FUNCTION__, ix, ap->port_no); +ata_dev_printk (adev,KERN_DEBUG ,"%s: ix = %d, port#: %d\n" , + __FUNCTION__, adev->devno, ap->port_no);/* Don't continue if not a SATA device. */ if (!(ap->cbl ==ATA_CBL_SATA )) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: Not a SATA device\n" , __FUNCTION__); goto out; }
@@ -655,25 +653,25 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix) err =sata_get_dev_handle (dev, &handle, &pcidevfn);if (err <0 ) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: sata_get_dev_handle failed (%d\n" , __FUNCTION__, err); goto out; }/* Get this drive's _ADR info, if not already known */ -if (!atadev->obj_handle) { +if (!adev->obj_handle) { dev_adr =SATA_ADR_RSVD ; - err =get_sata_adr (dev, handle, pcidevfn, ix, ap, atadev, + err =get_sata_adr (dev, handle, pcidevfn, adev->devno, ap, adev, &dev_adr);if (err <0 || dev_adr ==SATA_ADR_RSVD || - !atadev->obj_handle) { + !adev->obj_handle) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s: get_sata_adr failed: " "err=%d, dev_adr=%u, obj_handle=0x%p\n" , __FUNCTION__, err, dev_adr, - atadev->obj_handle); + adev->obj_handle); goto out; } }
@@ -683,19 +681,19 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix) input.count =1 ; input.pointer = in_params; in_params[0 ].type =ACPI_TYPE_BUFFER ; - in_params[0 ].buffer.length =sizeof (atadev->id[0 ]) *ATA_ID_WORDS ; - in_params[0 ].buffer.pointer = (u8 *)atadev->id; + in_params[0 ].buffer.length =sizeof (adev->id[0 ]) *ATA_ID_WORDS ; + in_params[0 ].buffer.pointer = (u8 *)adev->id;/* Output buffer: _SDD has no output */ /* It's OK for _SDD to be missing too. */ -swap_buf_le16 (atadev->id,ATA_ID_WORDS ); - status =acpi_evaluate_object (atadev->obj_handle,"_SDD" , &input,NULL ); -swap_buf_le16 (atadev->id,ATA_ID_WORDS ); +swap_buf_le16 (adev->id,ATA_ID_WORDS ); + status =acpi_evaluate_object (adev->obj_handle,"_SDD" , &input,NULL ); +swap_buf_le16 (adev->id,ATA_ID_WORDS ); err =ACPI_FAILURE (status) ? -EIO :0 ;if (err <0 ) {if (ata_msg_probe (ap)) -ata_dev_printk (atadev,KERN_DEBUG , +ata_dev_printk (adev,KERN_DEBUG ,"%s _SDD error: status = 0x%x\n" , __FUNCTION__, status); }
drivers/ata/libata-core.c +1 -1
@@ -1812,7 +1812,7 @@ int ata_dev_configure(struct ata_device *dev)ata_dev_printk (dev,KERN_DEBUG ,"%s: ENTER\n" , __FUNCTION__);/* set _SDD */ - rc =ata_acpi_push_id (ap, dev->devno); + rc =ata_acpi_push_id (dev);if (rc) {ata_dev_printk (dev,KERN_WARNING ,"failed to set _SDD(%d)\n" , rc);
drivers/ata/libata.h +2 -2
@@ -99,13 +99,13 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);/* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI extern int ata_acpi_exec_tfs (struct ata_port *ap); -extern int ata_acpi_push_id (struct ata_port *ap,unsigned int ix); +extern int ata_acpi_push_id (struct ata_device *adev);#else static inline int ata_acpi_exec_tfs (struct ata_port *ap) {return 0 ; } -static inline int ata_acpi_push_id (struct ata_port *ap,unsigned int ix) +static inline int ata_acpi_push_id (struct ata_device *adev) {return 0 ; }
Signature · 1 line
1.5.0.3
PATCH 7/13 libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Tejun HeoWhether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA, or it supports slave device or not. This patch adds ATA_FLAG_ACPI_SATA port flags which tells libata-acpi that the port needs SATA ACPI nodes, and sets the flag for ahci and sata_sil24.
--- drivers/ata/ahci.c | 3 ++- drivers/ata/libata-acpi.c | 10 +++++----- drivers/ata/sata_sil24.c | 3 ++- include/linux/libata.h | 1 + 4 files changed, 10 insertions(+), 7 deletions(-)
drivers/ata/ahci.c +2 -1
@@ -173,7 +173,8 @@ enum {AHCI_FLAG_COMMON =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY |ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | - ATA_FLAG_SKIP_D2H_BSY, +ATA_FLAG_SKIP_D2H_BSY | + ATA_FLAG_ACPI_SATA, };struct ahci_cmd_hdr {
drivers/ata/libata-acpi.c +5 -5
@@ -321,7 +321,7 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length,/* Don't continue if device has no _ADR method. * _GTF is intended for known motherboard devices. */ -if (!(ap->cbl ==ATA_CBL_SATA )) { +if (!(ap->flags &ATA_FLAG_ACPI_SATA )) { err =pata_get_dev_handle (dev, &dev_handle, &pcidevfn);if (err <0 ) {if (ata_msg_probe (ap))
@@ -343,7 +343,7 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length,/* Get this drive's _ADR info. if not already known. */ if (!adev->obj_handle) { -if (!(ap->cbl ==ATA_CBL_SATA )) { +if (!(ap->flags &ATA_FLAG_ACPI_SATA )) {/* get child objects of dev_handle == channel objects, * + _their_ children == drive objects */ /* channel is ap->port_no */
@@ -528,7 +528,7 @@ static int do_drive_set_taskfiles(struct ata_device *adev,ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , __FUNCTION__, ap->port_no); -if (libata_noacpi || !(ap->cbl ==ATA_CBL_SATA )) +if (libata_noacpi || !(ap->flags &ATA_FLAG_ACPI_SATA ))return 0 ;if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED ))
@@ -578,7 +578,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)* we should not run GTF on PATA devices since some * PATA require execution of GTM/STM before GTF. */ -if (!(ap->cbl ==ATA_CBL_SATA )) +if (!(ap->flags &ATA_FLAG_ACPI_SATA ))return 0 ;for (ix =0 ; ix <ATA_MAX_DEVICES ; ix++) {
@@ -641,7 +641,7 @@ int ata_acpi_push_id(struct ata_device *adev) __FUNCTION__, adev->devno, ap->port_no);/* Don't continue if not a SATA device. */ -if (!(ap->cbl ==ATA_CBL_SATA )) { +if (!(ap->flags &ATA_FLAG_ACPI_SATA )) {if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: Not a SATA device\n" , __FUNCTION__);
drivers/ata/sata_sil24.c +2 -1
@@ -237,7 +237,8 @@ enum {/* host flags */ SIL24_COMMON_FLAGS =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY |ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -ATA_FLAG_NCQ |ATA_FLAG_SKIP_D2H_BSY , +ATA_FLAG_NCQ |ATA_FLAG_SKIP_D2H_BSY | +ATA_FLAG_ACPI_SATA ,SIL24_FLAG_PCIX_IRQ_WOC = (1 <<24 ),/* IRQ loss errata on PCI-X */ IRQ_STAT_4PORTS = 0xf,
include/linux/libata.h +1 -0
@@ -174,6 +174,7 @@ enum {ATA_FLAG_SETXFER_POLLING = (1 <<14 ),/* use polling for SETXFER */ ATA_FLAG_IGN_SIMPLEX = (1 <<15 ),/* ignore SIMPLEX */ ATA_FLAG_NO_IORDY = (1 <<16 ),/* controller lacks iordy */ +ATA_FLAG_ACPI_SATA = (1 <<17 ),/* need native SATA ACPI layout */ /* The following flag belongs to ap->pflags but is kept in * ap->flags because it's referenced in many LLDs and will be
Signature · 1 line
1.5.0.3
PATCH 8/13 libata-acpi: implement ata_acpi_associate()
In reply to Tejun Heo* Add acpi_handle to ata_host and ata_port. Rename ata_device->obj_handle to ->acpi_handle and move it above such that it doesn't get cleared on reconfiguration.
* Replace ACPI node association which ata_acpi_associate() which is called once during host initialization. Unlike the previous implementation, ata_acpi_associate() uses ATA_FLAG_ACPI_SATA to choose between IDE or SATA ACPI hierarchy and uses simple child look up instead of recursive walk to match the nodes. This is way safer and simpler. Please read the following message for more info.
http://article.gmane.org/gmane.linux.ide/17554
--- drivers/ata/libata-acpi.c | 369 ++++++--------------------------------------- drivers/ata/libata-core.c | 3 + drivers/ata/libata.h | 2 + include/linux/libata.h | 13 +- 4 files changed, 63 insertions(+), 324 deletions(-)
drivers/ata/libata-acpi.c +49 -320
@@ -24,10 +24,8 @@#include <acpi/acmacros.h> #include <acpi/actypes.h> -#define SATA_ROOT_PORT (x) (((x) >> 16) & 0xffff) -#define SATA_PORT_NUMBER (x) ((x) & 0xffff)/* or NO_PORT_MULT */ #define NO_PORT_MULT 0xffff -#define SATA_ADR_RSVD 0xffffffff +#define SATA_ADR (root,pmp) (((root) << 16) | (pmp))#define REGS_PER_GTF 7struct taskfile_array {
@@ -42,230 +40,64 @@ static int is_pci_dev(struct device *dev)return (dev->bus == &pci_bus_type); } -/** -* sata_get_dev_handle - finds acpi_handle and PCI device.function -* @dev: device to locate -* @handle: returned acpi_handle for @dev -* @pcidevfn: return PCI device.func for @dev -* -* This function is somewhat SATA-specific. Or at least the -* PATA & SATA versions of this function are different, -* so it's not entirely generic code. -* -* Returns 0 on success, <0 on error. -*/ -static int sata_get_dev_handle (struct device *dev,acpi_handle *handle, -acpi_integer *pcidevfn) +static void ata_acpi_associate_sata_port (struct ata_port *ap) { -struct pci_dev *pci_dev; -acpi_integer addr; - -if (!is_pci_dev (dev)) -return -ENODEV ; - - pci_dev =to_pci_dev (dev);/* NOTE: PCI-specific */ -/* Please refer to the ACPI spec for the syntax of _ADR. */ - addr = (PCI_SLOT (pci_dev->devfn) <<16 ) |PCI_FUNC (pci_dev->devfn); - *pcidevfn = addr; - *handle =acpi_get_child (DEVICE_ACPI_HANDLE (dev->parent), addr); -if (!*handle) -return -ENODEV ; -return 0 ; +acpi_integer adr =SATA_ADR (ap->port_no,NO_PORT_MULT ); + + ap->device->acpi_handle =acpi_get_child (ap->host->acpi_handle, adr); } -/** -* pata_get_dev_handle - finds acpi_handle and PCI device.function -* @dev: device to locate -* @handle: returned acpi_handle for @dev -* @pcidevfn: return PCI device.func for @dev -* -* The PATA and SATA versions of this function are different. -* -* Returns 0 on success, <0 on error. -*/ -static int pata_get_dev_handle (struct device *dev,acpi_handle *handle, -acpi_integer *pcidevfn) +static void ata_acpi_associate_ide_port (struct ata_port *ap) { -unsigned int bus, devnum, func; -acpi_integer addr; -acpi_handle dev_handle, parent_handle; -struct acpi_buffer buffer = {.length =ACPI_ALLOCATE_BUFFER , - .pointer =NULL }; -acpi_status status; -struct acpi_device_info *dinfo =NULL ; -int ret = -ENODEV ; -struct pci_dev *pdev; - -if (!is_pci_dev (dev)) -return -ENODEV ; - - pdev =to_pci_dev (dev); - - bus = pdev->bus->number; - devnum =PCI_SLOT (pdev->devfn); - func =PCI_FUNC (pdev->devfn); - - dev_handle =DEVICE_ACPI_HANDLE (dev); - parent_handle =DEVICE_ACPI_HANDLE (dev->parent); - - status =acpi_get_object_info (parent_handle, &buffer); -if (ACPI_FAILURE (status)) - goto err; - - dinfo = buffer.pointer; -if (dinfo && (dinfo->valid &ACPI_VALID_ADR ) && - dinfo->address == bus) { -/* ACPI spec for _ADR for PCI bus: */ - addr = (acpi_integer)(devnum <<16 | func); - *pcidevfn = addr; - *handle = dev_handle; - }else { - goto err; - } +int max_devices, i; -if (!*handle) - goto err; - ret =0 ; -err: -kfree (dinfo); -return ret; -} - -struct walk_info {/* can be trimmed some */ -struct device *dev; -struct acpi_device *adev; -acpi_handle handle; -acpi_integer pcidevfn; -unsigned int drivenum; -acpi_handle obj_handle; -struct ata_port *ataport; -struct ata_device *atadev; -u32 sata_adr; -int status; -char basepath[ACPI_PATHNAME_MAX ]; -int basepath_len; -}; + ap->acpi_handle =acpi_get_child (ap->host->acpi_handle, ap->port_no); +if (!ap->acpi_handle) +return ; -static acpi_status get_devices (acpi_handle handle, -u32 level,void *context,void **return_value) -{ -acpi_status status; -struct walk_info *winfo = context; -struct acpi_buffer namebuf = {ACPI_ALLOCATE_BUFFER ,NULL }; -char *pathname; -struct acpi_buffer buffer; -struct acpi_device_info *dinfo; - - status =acpi_get_name (handle,ACPI_FULL_PATHNAME , &namebuf); -if (status) - goto ret; - pathname = namebuf.pointer; - - buffer.length =ACPI_ALLOCATE_BUFFER ; - buffer.pointer =NULL ; - status =acpi_get_object_info (handle, &buffer); -if (ACPI_FAILURE (status)) - goto out2; - - dinfo = buffer.pointer; - -/* find full device path name for pcidevfn */ -if (dinfo && (dinfo->valid &ACPI_VALID_ADR ) && - dinfo->address == winfo->pcidevfn) { -if (ata_msg_probe (winfo->ataport)) -ata_dev_printk (winfo->atadev,KERN_DEBUG , -":%s: matches pcidevfn (0x%llx)\n" , - pathname, winfo->pcidevfn); -strlcpy (winfo->basepath, pathname, -sizeof (winfo->basepath)); - winfo->basepath_len =strlen (pathname); - goto out; - } + max_devices =1 ; +if (ap->flags &ATA_FLAG_SLAVE_POSS ) + max_devices++; -/* if basepath is not yet known, ignore this object */ -if (!winfo->basepath_len) - goto out; +for (i =0 ; i < max_devices; i++) { +struct ata_device *adev = &ap->device[i]; -/* if this object is in scope of basepath, maybe use it */ -if (strncmp (pathname, winfo->basepath, - winfo->basepath_len) ==0 ) { -if (!(dinfo->valid &ACPI_VALID_ADR )) - goto out; -if (ata_msg_probe (winfo->ataport)) -ata_dev_printk (winfo->atadev,KERN_DEBUG , -"GOT ONE: (%s) root_port = 0x%llx," -" port_num = 0x%llx\n" , pathname, -SATA_ROOT_PORT (dinfo->address), -SATA_PORT_NUMBER (dinfo->address)); -/* heuristics: */ -if (SATA_PORT_NUMBER (dinfo->address) !=NO_PORT_MULT ) -if (ata_msg_probe (winfo->ataport)) -ata_dev_printk (winfo->atadev, -KERN_DEBUG ,"warning: don't" -" know how to handle SATA port" -" multiplier\n" ); -if (SATA_ROOT_PORT (dinfo->address) == - winfo->ataport->port_no && -SATA_PORT_NUMBER (dinfo->address) ==NO_PORT_MULT ) { -if (ata_msg_probe (winfo->ataport)) -ata_dev_printk (winfo->atadev, -KERN_DEBUG , -"THIS ^^^^^ is the requested" -" SATA drive (handle = 0x%p)\n" , - handle); - winfo->sata_adr = dinfo->address; - winfo->obj_handle = handle; - } + adev->acpi_handle =acpi_get_child (ap->acpi_handle, i); } -out: -kfree (dinfo); -out2: -kfree (pathname); - -ret: -return status; } -/* Get the SATA drive _ADR object. */ -static int get_sata_adr (struct device *dev,acpi_handle handle, -acpi_integer pcidevfn,unsigned int drive, -struct ata_port *ap, -struct ata_device *atadev,u32 *dev_adr) +/** +* ata_acpi_associate - associate ATA host with ACPI objects +* @host: target ATA host +* +* Look up ACPI objects associated with @host and initialize +* acpi_handle fields of @host, its ports and devices accordingly. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ +void ata_acpi_associate (struct ata_host *host) { -acpi_status status; -struct walk_info *winfo; -int err = -ENOMEM ; +int i; - winfo =kzalloc (sizeof (struct walk_info ),GFP_KERNEL ); -if (!winfo) - goto out; +if (!is_pci_dev (host->dev) || libata_noacpi) +return ; - winfo->dev = dev; - winfo->atadev = atadev; - winfo->ataport = ap; -if (acpi_bus_get_device (handle, &winfo->adev) <0 ) -if (ata_msg_probe (ap)) -ata_dev_printk (winfo->atadev,KERN_DEBUG , -"acpi_bus_get_device failed\n" ); - winfo->handle = handle; - winfo->pcidevfn = pcidevfn; - winfo->drivenum = drive; + host->acpi_handle =DEVICE_ACPI_HANDLE (host->dev); +if (!host->acpi_handle) +return ; - status =acpi_get_devices (NULL , get_devices, winfo,NULL ); -if (ACPI_FAILURE (status)) { -if (ata_msg_probe (ap)) -ata_dev_printk (winfo->atadev,KERN_DEBUG , -"%s: acpi_get_devices failed\n" , - __FUNCTION__); - err = -ENODEV ; - }else { - *dev_adr = winfo->sata_adr; - atadev->obj_handle = winfo->obj_handle; - err =0 ; +for (i =0 ; i < host->n_ports; i++) { +struct ata_port *ap = host->ports[i]; + +if (host->ports[0 ]->flags &ATA_FLAG_ACPI_SATA ) +ata_acpi_associate_sata_port (ap); +else +ata_acpi_associate_ide_port (ap); } -kfree (winfo); -out: -return err; }/**
@@ -290,20 +122,15 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length, {struct ata_port *ap = adev->ap;acpi_status status; -acpi_handle dev_handle =NULL ; -acpi_handle chan_handle, drive_handle; -acpi_integer pcidevfn =0 ; -u32 dev_adr;struct acpi_buffer output;union acpi_object *out_obj; -struct device *dev = ap->host->dev;int err = -ENODEV ; *gtf_length =0 ; *gtf_address =0UL ; *obj_loc =0UL ; -if (libata_noacpi) +if (!adev->acpi_handle)return 0 ;if (ata_msg_probe (ap))
@@ -319,78 +146,14 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length, goto out; } -/* Don't continue if device has no _ADR method. -* _GTF is intended for known motherboard devices. */ -if (!(ap->flags &ATA_FLAG_ACPI_SATA )) { - err =pata_get_dev_handle (dev, &dev_handle, &pcidevfn); -if (err <0 ) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: pata_get_dev_handle failed (%d)\n" , - __FUNCTION__, err); - goto out; - } - }else { - err =sata_get_dev_handle (dev, &dev_handle, &pcidevfn); -if (err <0 ) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: sata_get_dev_handle failed (%d\n" , - __FUNCTION__, err); - goto out; - } - } - -/* Get this drive's _ADR info. if not already known. */ -if (!adev->obj_handle) { -if (!(ap->flags &ATA_FLAG_ACPI_SATA )) { -/* get child objects of dev_handle == channel objects, -* + _their_ children == drive objects */ -/* channel is ap->port_no */ - chan_handle =acpi_get_child (dev_handle, - ap->port_no); -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: chan adr=%d: chan_handle=0x%p\n" , - __FUNCTION__, ap->port_no, - chan_handle); -if (!chan_handle) { - err = -ENODEV ; - goto out; - } -/* TBD: could also check ACPI object VALID bits */ - drive_handle =acpi_get_child (chan_handle, adev->devno); -if (!drive_handle) { - err = -ENODEV ; - goto out; - } - dev_adr = adev->devno; - adev->obj_handle = drive_handle; - }else {/* for SATA mode */ - dev_adr =SATA_ADR_RSVD ; - err =get_sata_adr (dev, dev_handle, pcidevfn,0 , - ap, adev, &dev_adr); - } -if (err <0 || dev_adr ==SATA_ADR_RSVD || - !adev->obj_handle) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: get_sata/pata_adr failed: " -"err=%d, dev_adr=%u, obj_handle=0x%p\n" , - __FUNCTION__, err, dev_adr, - adev->obj_handle); - goto out; - } - } -/* Setting up output buffer */ output.length =ACPI_ALLOCATE_BUFFER ; output.pointer =NULL ;/* ACPI-CA sets this; save/free it later */ /* _GTF has no input parameters */ err = -EIO ; - status =acpi_evaluate_object (adev->obj_handle,"_GTF" , -NULL , &output); + status =acpi_evaluate_object (adev->acpi_handle,"_GTF" , +NULL , &output);if (ACPI_FAILURE (status)) {if (ata_msg_probe (ap)) ata_dev_printk(adev, KERN_DEBUG,
@@ -528,7 +291,7 @@ static int do_drive_set_taskfiles(struct ata_device *adev,ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , __FUNCTION__, ap->port_no); -if (libata_noacpi || !(ap->flags &ATA_FLAG_ACPI_SATA )) +if (!(ap->flags &ATA_FLAG_ACPI_SATA ))return 0 ;if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED ))
@@ -571,8 +334,6 @@ int ata_acpi_exec_tfs(struct ata_port *ap)unsigned long gtf_address;unsigned long obj_loc; -if (libata_noacpi) -return 0 ;/* * TBD - implement PATA support. For now, * we should not run GTF on PATA devices since some
@@ -624,16 +385,12 @@ int ata_acpi_exec_tfs(struct ata_port *ap)int ata_acpi_push_id (struct ata_device *adev) {struct ata_port *ap = adev->ap; -acpi_handle handle; -acpi_integer pcidevfn;int err; -struct device *dev = ap->host->dev; -u32 dev_adr;acpi_status status;struct acpi_object_list input;union acpi_object in_params[1 ]; -if (libata_noacpi) +if (!adev->acpi_handle)return 0 ;if (ata_msg_probe (ap))
@@ -648,34 +405,6 @@ int ata_acpi_push_id(struct ata_device *adev) goto out; } -/* Don't continue if device has no _ADR method. -* _SDD is intended for known motherboard devices. */ - err =sata_get_dev_handle (dev, &handle, &pcidevfn); -if (err <0 ) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: sata_get_dev_handle failed (%d\n" , - __FUNCTION__, err); - goto out; - } - -/* Get this drive's _ADR info, if not already known */ -if (!adev->obj_handle) { - dev_adr =SATA_ADR_RSVD ; - err =get_sata_adr (dev, handle, pcidevfn, adev->devno, ap, adev, - &dev_adr); -if (err <0 || dev_adr ==SATA_ADR_RSVD || - !adev->obj_handle) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: get_sata_adr failed: " -"err=%d, dev_adr=%u, obj_handle=0x%p\n" , - __FUNCTION__, err, dev_adr, - adev->obj_handle); - goto out; - } - } -/* Give the drive Identify data to the drive via the _SDD method */ /* _SDD: set up input parameters */ input.count =1 ;
@@ -687,7 +416,7 @@ int ata_acpi_push_id(struct ata_device *adev)/* It's OK for _SDD to be missing too. */ swap_buf_le16 (adev->id,ATA_ID_WORDS ); - status =acpi_evaluate_object (adev->obj_handle,"_SDD" , &input,NULL ); + status =acpi_evaluate_object (adev->acpi_handle,"_SDD" , &input,NULL );swap_buf_le16 (adev->id,ATA_ID_WORDS ); err =ACPI_FAILURE (status) ? -EIO :0 ;
drivers/ata/libata-core.c +3 -0
@@ -6223,6 +6223,9 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)if (rc)return rc; +/* associate with ACPI nodes */ +ata_acpi_associate (host); +/* set cable, sata_spd_limit and report */ for (i =0 ; i < host->n_ports; i++) {struct ata_port *ap = host->ports[i];
drivers/ata/libata.h +2 -0
@@ -98,9 +98,11 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);/* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI +extern void ata_acpi_associate (struct ata_host *host);extern int ata_acpi_exec_tfs (struct ata_port *ap);extern int ata_acpi_push_id (struct ata_device *adev);#else +static inline void ata_acpi_associate (struct ata_host *host) { }static inline int ata_acpi_exec_tfs (struct ata_port *ap) {return 0 ;
include/linux/libata.h +9 -4
@@ -381,6 +381,9 @@ struct ata_host {void *private_data;const struct ata_port_operations *ops;unsigned long flags; +#ifdef CONFIG_ATA_ACPI +acpi_handle acpi_handle; +#endif struct ata_port *simplex_claimed;/* channel owning the DMA */ struct ata_port *ports[0 ]; };
@@ -447,6 +450,9 @@ struct ata_device {unsigned int devno;/* 0 or 1 */ unsigned long flags;/* ATA_DFLAG_xxx */ struct scsi_device *sdev;/* attached SCSI device */ +#ifdef CONFIG_ATA_ACPI +acpi_handle acpi_handle; +#endif /* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */ u64 n_sectors;/* size of device, if ATA */ unsigned int class;/* ATA_DEV_xxx */
@@ -475,10 +481,6 @@ struct ata_device {struct ata_ering ering;int spdn_cnt;unsigned int horkage;/* List of broken features */ -#ifdef CONFIG_ATA_ACPI -/* ACPI objects info */ -acpi_handle obj_handle; -#endif };/* Offset into struct ata_device. Fields above it are maintained
@@ -568,6 +570,9 @@ struct ata_port {void *private_data; +#ifdef CONFIG_ATA_ACPI +acpi_handle acpi_handle; +#endif u8 sector_buf[ATA_SECT_SIZE ];/* owned by EH */ };
Signature · 1 line
1.5.0.3
PATCH 9/13 libata-acpi: clean up ata_acpi_exec_tfs()
In reply to Tejun HeoThis patch cleans up ata_acpi_exec_tfs() and its friends.
* Rename taskfile_array to ata_acpi_gtf and make it __packed as it's used as argument to ACPI method, and use pointer to ata_acpi_gtf and number of taskfiles to represent _GTF taskfiles instead of a pointer casted into unsigned long and byte count. This makes argument re-checking in do_drive_set_taskfiles() unnecessary.
* Pointer in void * not in unsigned long.
* Clean up do_drive_get_GTF() error handling and make do_drive_get_GTF() return number of taskfiles on success, 0 if _GTF doesn't exist or doesn't contain valid ata. -errno on other errors.
* Remove superflous check for acpi->buffer.pointer.
* Update taskfile_load_raw() such that printed messages look similar to the messages printed by ata_eh_report().
--- drivers/ata/libata-acpi.c | 219 ++++++++++++++++++++++----------------------- 1 files changed, 107 insertions(+), 112 deletions(-)
drivers/ata/libata-acpi.c +107 -112
@@ -28,9 +28,9 @@#define SATA_ADR (root,pmp) (((root) << 16) | (pmp))#define REGS_PER_GTF 7 -struct taskfile_array { -u8 tfa[REGS_PER_GTF ];/* regs. 0x1f1 - 0x1f7 */ -}; +struct ata_acpi_gtf { +u8 tf[REGS_PER_GTF ];/* regs. 0x1f1 - 0x1f7 */ +} __packed;/* * Helper - belongs in the PCI layer somewhere eventually
@@ -103,8 +103,8 @@ void ata_acpi_associate(struct ata_host *host)/** * do_drive_get_GTF - get the drive bootup default taskfile settings * @adev: target ATA device -* @gtf_length: number of bytes of _GTF data returned at @gtf_address -* @gtf_address: buffer containing _GTF taskfile arrays +* @gtf: output parameter for buffer containing _GTF taskfile arrays +* @ptr_to_free: pointer which should be freed * * This applies to both PATA and SATA drives. *
@@ -114,24 +114,28 @@ void ata_acpi_associate(struct ata_host *host)* The <variable number> is not known in advance, so have ACPI-CA * allocate the buffer as needed and return it, then free it later. * -* The returned @gtf_length and @gtf_address are only valid if the -* function return value is 0. +* LOCKING: +* EH context. +* +* RETURNS: +* Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't +* contain valid data. -errno on other errors. */ -static int do_drive_get_GTF (struct ata_device *adev,unsigned int *gtf_length, -unsigned long *gtf_address,unsigned long *obj_loc) +static int do_drive_get_GTF (struct ata_device *adev,struct ata_acpi_gtf **gtf, +void **ptr_to_free) {struct ata_port *ap = adev->ap;acpi_status status;struct acpi_buffer output;union acpi_object *out_obj; -int err = -ENODEV ; +int rc =0 ; - *gtf_length =0 ; - *gtf_address =0UL ; - *obj_loc =0UL ; +/* set up output buffer */ + output.length =ACPI_ALLOCATE_BUFFER ; + output.pointer =NULL ;/* ACPI-CA sets this; save/free it later */ if (!adev->acpi_handle) -return 0 ; + goto out_free;if (ata_msg_probe (ap)) ata_dev_printk(adev, KERN_DEBUG, "%s: ENTER: port#: %d\n",
@@ -143,23 +147,19 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length,"ata_dev_present: %d, PORT_DISABLED: %lu\n" , __FUNCTION__,ata_dev_enabled (adev), ap->flags & ATA_FLAG_DISABLED); - goto out; + goto out_free; } -/* Setting up output buffer */ - output.length =ACPI_ALLOCATE_BUFFER ; - output.pointer =NULL ;/* ACPI-CA sets this; save/free it later */ -/* _GTF has no input parameters */ - err = -EIO ; - status =acpi_evaluate_object (adev->acpi_handle,"_GTF" , -NULL , &output); + status =acpi_evaluate_object (adev->acpi_handle,"_GTF" ,NULL , &output);if (ACPI_FAILURE (status)) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: Run _GTF error: status = 0x%x\n" , - __FUNCTION__, status); - goto out; +if (status !=AE_NOT_FOUND ) { +ata_dev_printk (adev,KERN_WARNING , +"_GTF evaluation failed (AE 0x%x)\n" , + status); + rc = -EIO ; + } + goto out_free; }if (!output.length || !output.pointer) {
@@ -169,43 +169,43 @@ static int do_drive_get_GTF(struct ata_device *adev, unsigned int *gtf_length, __FUNCTION__, (unsigned long long )output.length, output.pointer); -kfree (output.pointer); - goto out; + goto out_free; } out_obj = output.pointer;if (out_obj->type !=ACPI_TYPE_BUFFER ) { -kfree (output.pointer);if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: Run _GTF: " "error: expected object type of " " ACPI_TYPE_BUFFER, got 0x%x\n" , __FUNCTION__, out_obj->type); - err = -ENOENT ; - goto out; + rc = -EINVAL ; + goto out_free; } -if (!out_obj->buffer.length || !out_obj->buffer.pointer || - out_obj->buffer.length %REGS_PER_GTF ) { +if (out_obj->buffer.length %REGS_PER_GTF ) {if (ata_msg_drv (ap))ata_dev_printk (adev,KERN_ERR ,"%s: unexpected GTF length (%d) or addr (0x%p)\n" , __FUNCTION__, out_obj->buffer.length, out_obj->buffer.pointer); - err = -ENOENT ; - goto out; + rc = -EINVAL ; + goto out_free; } - *gtf_length = out_obj->buffer.length; - *gtf_address = (unsigned long )out_obj->buffer.pointer; - *obj_loc = (unsigned long )out_obj; + *ptr_to_free = out_obj; + *gtf = (void *)out_obj->buffer.pointer; + rc = out_obj->buffer.length /REGS_PER_GTF ; +if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: returning " -"gtf_length=%d, gtf_address=0x%lx, obj_loc=0x%lx\n" , - __FUNCTION__, *gtf_length, *gtf_address, *obj_loc); - err =0 ; -out: -return err; +"gtf=%p, gtf_count=%d, ptr_to_free=%p\n" , + __FUNCTION__, *gtf, rc, *ptr_to_free); +return rc; + + out_free: +kfree (output.pointer); +return rc; }/**
@@ -224,68 +224,78 @@ out:* function also waits for idle after writing control and before * writing the remaining registers. * -* LOCKING: TBD: -* Inherited from caller. +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. */ -static void taskfile_load_raw (struct ata_device *adev, -const struct taskfile_array *gtf) +static int taskfile_load_raw (struct ata_device *adev, +const struct ata_acpi_gtf *gtf) {struct ata_port *ap = adev->ap; -struct ata_taskfile tf; -unsigned int err; +struct ata_taskfile tf, rtf; +unsigned int err_mask; -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG ,"%s: (0x1f1-1f7): hex: " -"%02x %02x %02x %02x %02x %02x %02x\n" , - __FUNCTION__, - gtf->tfa[0 ], gtf->tfa[1 ], gtf->tfa[2 ], - gtf->tfa[3 ], gtf->tfa[4 ], gtf->tfa[5 ], gtf->tfa[6 ]); - -if ((gtf->tfa[0 ] ==0 ) && (gtf->tfa[1 ] ==0 ) && (gtf->tfa[2 ] ==0 ) - && (gtf->tfa[3 ] ==0 ) && (gtf->tfa[4 ] ==0 ) && (gtf->tfa[5 ] ==0 ) - && (gtf->tfa[6 ] ==0 )) -return ; +if ((gtf->tf[0 ] ==0 ) && (gtf->tf[1 ] ==0 ) && (gtf->tf[2 ] ==0 ) + && (gtf->tf[3 ] ==0 ) && (gtf->tf[4 ] ==0 ) && (gtf->tf[5 ] ==0 ) + && (gtf->tf[6 ] ==0 )) +return 0 ;ata_tf_init (adev, &tf);/* convert gtf to tf */ tf.flags |=ATA_TFLAG_ISADDR |ATA_TFLAG_DEVICE ;/* TBD */ tf.protocol =ATA_PROT_NODATA ; - tf.feature = gtf->tfa[0 ];/* 0x1f1 */ - tf.nsect = gtf->tfa[1 ];/* 0x1f2 */ - tf.lbal = gtf->tfa[2 ];/* 0x1f3 */ - tf.lbam = gtf->tfa[3 ];/* 0x1f4 */ - tf.lbah = gtf->tfa[4 ];/* 0x1f5 */ - tf.device = gtf->tfa[5 ];/* 0x1f6 */ - tf.command = gtf->tfa[6 ];/* 0x1f7 */ - - err =ata_exec_internal (adev, &tf,NULL ,DMA_NONE ,NULL ,0 ); -if (err &&ata_msg_probe (ap)) + tf.feature = gtf->tf[0 ];/* 0x1f1 */ + tf.nsect = gtf->tf[1 ];/* 0x1f2 */ + tf.lbal = gtf->tf[2 ];/* 0x1f3 */ + tf.lbam = gtf->tf[3 ];/* 0x1f4 */ + tf.lbah = gtf->tf[4 ];/* 0x1f5 */ + tf.device = gtf->tf[5 ];/* 0x1f6 */ + tf.command = gtf->tf[6 ];/* 0x1f7 */ + +if (ata_msg_probe (ap)) +ata_dev_printk (adev,KERN_DEBUG ,"executing ACPI cmd " +"%02x/%02x:%02x:%02x:%02x:%02x:%02x\n" , + tf.command, tf.feature, tf.nsect, + tf.lbal, tf.lbam, tf.lbah, tf.device); + + rtf = tf; + err_mask =ata_exec_internal (adev, &rtf,NULL ,DMA_NONE ,NULL ,0 ); +if (err_mask) {ata_dev_printk (adev,KERN_ERR , -"%s: ata_exec_internal failed: %u\n" , - __FUNCTION__, err); +"ACPI cmd %02x/%02x:%02x:%02x:%02x:%02x:%02x failed " +"(Emask=0x%x Stat=0x%02x Err=0x%02x)\n" , + tf.command, tf.feature, tf.nsect, tf.lbal, tf.lbam, + tf.lbah, tf.device, err_mask, rtf.command, rtf.feature); +return -EIO ; + } + +return 0 ; }/** * do_drive_set_taskfiles - write the drive taskfile settings from _GTF * @adev: target ATA device -* @gtf_length: total number of bytes of _GTF taskfiles -* @gtf_address: location of _GTF taskfile arrays +* @gtf: pointer to array of _GTF taskfiles to execute +* @gtf_count: number of taskfiles * * This applies to both PATA and SATA drives. * -* Write {gtf_address, length gtf_length} in groups of -* REGS_PER_GTF bytes. +* Execute taskfiles in @gtf. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. */ static int do_drive_set_taskfiles (struct ata_device *adev, -unsigned int gtf_length, -unsigned long gtf_address) +struct ata_acpi_gtf *gtf,int gtf_count) {struct ata_port *ap = adev->ap; -int err = -ENODEV ; -int gtf_count = gtf_length /REGS_PER_GTF ;int ix; -struct taskfile_array *gtf;if (ata_msg_probe (ap)) ata_dev_printk(adev, KERN_DEBUG, "%s: ENTER: port#: %d\n",
@@ -295,29 +305,13 @@ static int do_drive_set_taskfiles(struct ata_device *adev,return 0 ;if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED )) - goto out; -if (!gtf_count)/* shouldn't be here */ - goto out; - -if (gtf_length %REGS_PER_GTF ) { -if (ata_msg_drv (ap)) -ata_dev_printk (adev,KERN_ERR , -"%s: unexpected GTF length (%d)\n" , - __FUNCTION__, gtf_length); - goto out; - } +return -ENODEV ; -for (ix =0 ; ix < gtf_count; ix++) { - gtf = (struct taskfile_array *) - (gtf_address + ix *REGS_PER_GTF ); +/* send all TaskFile registers (0x1f1-0x1f7) *in*that*order* */ +for (ix =0 ; ix < gtf_count; ix++) +taskfile_load_raw (adev, gtf++); -/* send all TaskFile registers (0x1f1-0x1f7) *in*that*order* */ -taskfile_load_raw (adev, gtf); - } - - err =0 ; -out: -return err; +return 0 ; }/**
@@ -328,11 +322,7 @@ out:*/ int ata_acpi_exec_tfs (struct ata_port *ap) { -int ix; -int ret =0 ; -unsigned int gtf_length; -unsigned long gtf_address; -unsigned long obj_loc; +int ix, ret =0 ;/* * TBD - implement PATA support. For now,
@@ -344,12 +334,16 @@ int ata_acpi_exec_tfs(struct ata_port *ap)for (ix =0 ; ix <ATA_MAX_DEVICES ; ix++) {struct ata_device *adev = &ap->device[ix]; +struct ata_acpi_gtf *gtf =NULL ; +int gtf_count; +void *ptr_to_free =NULL ;if (!ata_dev_enabled (adev))continue ; - ret =do_drive_get_GTF (adev, >f_length, >f_address, - &obj_loc); + ret =do_drive_get_GTF (adev, >f, &ptr_to_free); +if (ret ==0 ) +continue ;if (ret <0 ) {if (ata_msg_probe (ap)) ata_port_printk(ap, KERN_DEBUG,
@@ -357,9 +351,10 @@ int ata_acpi_exec_tfs(struct ata_port *ap) __FUNCTION__, ret);break ; } + gtf_count = ret; - ret =do_drive_set_taskfiles (adev, gtf_length, gtf_address); -kfree ((void *)obj_loc); + ret =do_drive_set_taskfiles (adev, gtf, gtf_count); +kfree (ptr_to_free);if (ret <0 ) {if (ata_msg_probe (ap)) ata_port_printk(ap, KERN_DEBUG,
Signature · 1 line
1.5.0.3
PATCH 10/13 libata-acpi: miscellaneous cleanups
In reply to Tejun Heo* Add missing LOCKING: and RETURNS: to function comment.
* Don't conditionalize warning messages with ata_msg_probe(). Print directly with KERN_WARNING.
* Drop duplicate debug messages.
--- drivers/ata/libata-acpi.c | 51 ++++++++++++++++++++------------------------ 1 files changed, 23 insertions(+), 28 deletions(-)
drivers/ata/libata-acpi.c +23 -28
@@ -174,21 +174,17 @@ static int do_drive_get_GTF(struct ata_device *adev, struct ata_acpi_gtf **gtf, out_obj = output.pointer;if (out_obj->type !=ACPI_TYPE_BUFFER ) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG ,"%s: Run _GTF: " -"error: expected object type of " -" ACPI_TYPE_BUFFER, got 0x%x\n" , - __FUNCTION__, out_obj->type); +ata_dev_printk (adev,KERN_WARNING , +"_GTF unexpected object type 0x%x\n" , + out_obj->type); rc = -EINVAL ; goto out_free; }if (out_obj->buffer.length %REGS_PER_GTF ) { -if (ata_msg_drv (ap)) -ata_dev_printk (adev,KERN_ERR , -"%s: unexpected GTF length (%d) or addr (0x%p)\n" , - __FUNCTION__, out_obj->buffer.length, - out_obj->buffer.pointer); +ata_dev_printk (adev,KERN_WARNING , +"unexpected _GTF length (%d)\n" , + out_obj->buffer.length); rc = -EINVAL ; goto out_free; }
@@ -319,6 +315,12 @@ static int do_drive_set_taskfiles(struct ata_device *adev,* @ap: the ata_port for the drive * * This applies to both PATA and SATA drives. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. */ int ata_acpi_exec_tfs (struct ata_port *ap) {
@@ -344,24 +346,14 @@ int ata_acpi_exec_tfs(struct ata_port *ap) ret =do_drive_get_GTF (adev, >f, &ptr_to_free);if (ret ==0 )continue ; -if (ret <0 ) { -if (ata_msg_probe (ap)) -ata_port_printk (ap,KERN_DEBUG , -"%s: get_GTF error (%d)\n" , - __FUNCTION__, ret); +if (ret <0 )break ; - } gtf_count = ret; ret =do_drive_set_taskfiles (adev, gtf, gtf_count);kfree (ptr_to_free); -if (ret <0 ) { -if (ata_msg_probe (ap)) -ata_port_printk (ap,KERN_DEBUG , -"%s: set_taskfiles error (%d)\n" , - __FUNCTION__, ret); +if (ret <0 )break ; - } }return ret;
@@ -376,6 +368,12 @@ int ata_acpi_exec_tfs(struct ata_port *ap)* ATM this function never returns a failure. It is an optional * method and if it fails for whatever reason, we should still * just keep going. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. */ int ata_acpi_push_id (struct ata_device *adev) {
@@ -415,12 +413,9 @@ int ata_acpi_push_id(struct ata_device *adev)swap_buf_le16 (adev->id,ATA_ID_WORDS ); err =ACPI_FAILURE (status) ? -EIO :0 ; -if (err <0 ) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s _SDD error: status = 0x%x\n" , - __FUNCTION__, status); - } +if (err <0 ) +ata_dev_printk (adev,KERN_WARNING , +"ACPI _SDD failed (AE 0x%x)\n" , status);/* always return success */ out:
Signature · 1 line
1.5.0.3
PATCH 11/13 libata: reimplement ACPI invocation
In reply to Tejun HeoThis patch reimplements ACPI invocation such that, instead of exporting ACPI details to the rest of libata, ACPI event handlers - ata_acpi_on_resume() and ata_acpi_on_devcfg() - are used. These two functions are responsible for determining whether specific ACPI method is used and when.
On resume, _GTF is scheduled by setting ATA_DFLAG_ACPI_PENDING device flag. This is done this way to avoid performing the action on wrong device device (device swapping while suspended).
On every ata_dev_configure(), ata_acpi_on_devcfg() is called, which performs _SDD and _GTF. _GTF is performed only after resuming and, if SATA, hardreset as the ACPI spec specifies. As _GTF may contain arbitrary commands, IDENTIFY page is re-read after _GTF taskfiles are executed.
If one of ACPI methods fails, ata_acpi_on_devcfg() retries on the first failure. If it fails again on the second try, ACPI is disabled on the device. Note that successful configuration clears ACPI failed status.
With all feature checks moved to the above two functions, do_drive_set_taskfiles() is trivial and thus collapsed into ata_acpi_exec_tfs(), which is now static and converted to return the number of executed taskfiles to be used by ata_acpi_on_resume(). As failures are handled properly, ata_acpi_push_id() now returns -errno on errors instead of unconditional zero.
--- drivers/ata/libata-acpi.c | 204 +++++++++++++++++++++++++++----------------- drivers/ata/libata-core.c | 16 ++-- drivers/ata/libata-eh.c | 3 + drivers/ata/libata.h | 14 +-- include/linux/libata.h | 2 + 5 files changed, 140 insertions(+), 99 deletions(-)
drivers/ata/libata-acpi.c +125 -79
@@ -272,91 +272,47 @@ static int taskfile_load_raw(struct ata_device *adev, }/** -* do_drive_set_taskfiles - write the drive taskfile settings from _GTF -* @adev: target ATA device -* @gtf: pointer to array of _GTF taskfiles to execute -* @gtf_count: number of taskfiles -* -* This applies to both PATA and SATA drives. -* -* Execute taskfiles in @gtf. -* -* LOCKING: -* EH context. -* -* RETURNS: -* 0 on success, -errno on failure. -*/ -static int do_drive_set_taskfiles (struct ata_device *adev, -struct ata_acpi_gtf *gtf,int gtf_count) -{ -struct ata_port *ap = adev->ap; -int ix; - -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , - __FUNCTION__, ap->port_no); - -if (!(ap->flags &ATA_FLAG_ACPI_SATA )) -return 0 ; - -if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED )) -return -ENODEV ; - -/* send all TaskFile registers (0x1f1-0x1f7) *in*that*order* */ -for (ix =0 ; ix < gtf_count; ix++) -taskfile_load_raw (adev, gtf++); - -return 0 ; -} - -/** * ata_acpi_exec_tfs - get then write drive taskfile settings -* @ap: the ata_port for the drive +* @adev: target ATA device * -* This applies to both PATA and SATA drives. +* Evaluate _GTF and excute returned taskfiles. * * LOCKING: * EH context. * * RETURNS: -* 0 on success, -errno on failure. +* Number of executed taskfiles on success, 0 if _GTF doesn't exist or +* doesn't contain valid data. -errno on other errors. */ -int ata_acpi_exec_tfs (struct ata_port *ap) +static int ata_acpi_exec_tfs (struct ata_device *adev) { -int ix, ret =0 ; - -/* -* TBD - implement PATA support. For now, -* we should not run GTF on PATA devices since some -* PATA require execution of GTM/STM before GTF. -*/ -if (!(ap->flags &ATA_FLAG_ACPI_SATA )) -return 0 ; - -for (ix =0 ; ix <ATA_MAX_DEVICES ; ix++) { -struct ata_device *adev = &ap->device[ix]; -struct ata_acpi_gtf *gtf =NULL ; -int gtf_count; -void *ptr_to_free =NULL ; - -if (!ata_dev_enabled (adev)) -continue ; - - ret =do_drive_get_GTF (adev, >f, &ptr_to_free); -if (ret ==0 ) -continue ; -if (ret <0 ) -break ; - gtf_count = ret; - - ret =do_drive_set_taskfiles (adev, gtf, gtf_count); -kfree (ptr_to_free); -if (ret <0 ) -break ; +struct ata_acpi_gtf *gtf =NULL ; +void *ptr_to_free =NULL ; +int gtf_count, i, rc; + +/* get taskfiles */ + rc =do_drive_get_GTF (adev, >f, &ptr_to_free); +if (rc <0 ) +return rc; + gtf_count = rc; + +/* execute them */ +for (i =0 , rc =0 ; i < gtf_count; i++) { +int tmp; + +/* ACPI errors are eventually ignored. Run till the +* end even after errors. +*/ + tmp =taskfile_load_raw (adev, gtf++); +if (!rc) + rc = tmp; } -return ret; +kfree (ptr_to_free); + +if (rc ==0 ) +return gtf_count; +return rc; }/**
@@ -375,7 +331,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)* RETURNS: * 0 on success, -errno on failure. */ -int ata_acpi_push_id (struct ata_device *adev) +static int ata_acpi_push_id (struct ata_device *adev) {struct ata_port *ap = adev->ap;int err;
@@ -395,7 +351,7 @@ int ata_acpi_push_id(struct ata_device *adev)if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: Not a SATA device\n" , __FUNCTION__); - goto out; +return 0 ; }/* Give the drive Identify data to the drive via the _SDD method */
@@ -417,9 +373,99 @@ int ata_acpi_push_id(struct ata_device *adev)ata_dev_printk (adev,KERN_WARNING ,"ACPI _SDD failed (AE 0x%x)\n" , status); -/* always return success */ -out: -return 0 ; +return err; +} + +/** +* ata_acpi_on_resume - ATA ACPI hook called on resume +* @ap: target ATA port +* +* This function is called when @ap is resumed - right after port +* itself is resumed but before any EH action is taken. +* +* LOCKING: +* EH context. +*/ +void ata_acpi_on_resume (struct ata_port *ap) +{ +int i; + +/* schedule _GTF */ +for (i =0 ; i <ATA_MAX_DEVICES ; i++) + ap->device[i].flags |=ATA_DFLAG_ACPI_PENDING ; } +/** +* ata_acpi_on_devcfg - ATA ACPI hook called on device donfiguration +* @adev: target ATA device +* +* This function is called when @adev is about to be configured. +* IDENTIFY data might have been modified after this hook is run. +* +* LOCKING: +* EH context. +* +* RETURNS: +* Positive number if IDENTIFY data needs to be refreshed, 0 if not, +* -errno on failure. +*/ +int ata_acpi_on_devcfg (struct ata_device *adev) +{ +struct ata_port *ap = adev->ap; +struct ata_eh_context *ehc = &ap->eh_context; +int acpi_sata = ap->flags &ATA_FLAG_ACPI_SATA ; +int rc; + +/* XXX: _STM isn't implemented yet, skip if IDE for now */ +if (!acpi_sata) +return 0 ; +if (!adev->acpi_handle) +return 0 ; + +/* do we need to do _GTF? */ +if (!(adev->flags &ATA_DFLAG_ACPI_PENDING ) && + !(acpi_sata && (ehc->i.flags &ATA_EHI_DID_HARDRESET ))) +return 0 ; + +/* do _SDD if SATA */ +if (acpi_sata) { + rc =ata_acpi_push_id (adev); +if (rc) + goto acpi_err; + } + +/* do _GTF */ + rc =ata_acpi_exec_tfs (adev); +if (rc <0 ) + goto acpi_err; + + adev->flags &= ~ATA_DFLAG_ACPI_PENDING ; + +/* refresh IDENTIFY page if any _GTF command has been executed */ +if (rc >0 ) { + rc =ata_dev_reread_id (adev,0 ); +if (rc <0 ) { +ata_dev_printk (adev,KERN_ERR ,"failed to IDENTIFY " +"after ACPI commands\n" ); +return rc; + } + } + +return 0 ; + + acpi_err: +/* let EH retry on the first failure, disable ACPI on the second */ +if (adev->flags &ATA_DFLAG_ACPI_FAILED ) { +ata_dev_printk (adev,KERN_WARNING ,"ACPI on devcfg failed the " +"second time, disabling (errno=%d)\n" , rc); + + adev->acpi_handle =NULL ; + +/* if port is working, request IDENTIFY reload and continue */ +if (!(ap->pflags &ATA_PFLAG_FROZEN )) + rc =1 ; + } + adev->flags |=ATA_DFLAG_ACPI_FAILED ; +return rc; +}
drivers/ata/libata-core.c +6 -10
@@ -1794,7 +1794,8 @@ static void ata_dev_config_ncq(struct ata_device *dev,int ata_dev_configure (struct ata_device *dev) {struct ata_port *ap = dev->ap; -int print_info = ap->eh_context.i.flags &ATA_EHI_PRINTINFO ; +struct ata_eh_context *ehc = &ap->eh_context; +int print_info = ehc->i.flags &ATA_EHI_PRINTINFO ;const u16 *id = dev->id;unsigned int xfer_mask;char revbuf[7 ];/* XYZ-99\0 */
@@ -1811,15 +1812,10 @@ int ata_dev_configure(struct ata_device *dev)if (ata_msg_probe (ap))ata_dev_printk (dev,KERN_DEBUG ,"%s: ENTER\n" , __FUNCTION__); -/* set _SDD */ - rc =ata_acpi_push_id (dev); -if (rc) { -ata_dev_printk (dev,KERN_WARNING ,"failed to set _SDD(%d)\n" , - rc); - } - -/* retrieve and execute the ATA task file of _GTF */ -ata_acpi_exec_tfs (ap); +/* let ACPI work its magic */ + rc =ata_acpi_on_devcfg (dev); +if (rc) +return rc;/* print device capabilities */ if (ata_msg_probe (ap))
drivers/ata/libata-eh.c +3 -0
@@ -2399,6 +2399,9 @@ static void ata_eh_handle_port_resume(struct ata_port *ap)if (ap->ops->port_resume) rc = ap->ops->port_resume (ap); +/* tell ACPI that we're resuming */ +ata_acpi_on_resume (ap); +/* give devices time to request EH */ timeout = jiffies +HZ ;/* 1s max */ while (1 ) {
drivers/ata/libata.h +4 -10
@@ -99,18 +99,12 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);/* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI extern void ata_acpi_associate (struct ata_host *host); -extern int ata_acpi_exec_tfs (struct ata_port *ap); -extern int ata_acpi_push_id (struct ata_device *adev); +extern void ata_acpi_on_resume (struct ata_port *ap); +extern int ata_acpi_on_devcfg (struct ata_device *adev);#else static inline void ata_acpi_associate (struct ata_host *host) { } -static inline int ata_acpi_exec_tfs (struct ata_port *ap) -{ -return 0 ; -} -static inline int ata_acpi_push_id (struct ata_device *adev) -{ -return 0 ; -} +static inline void ata_acpi_on_resume (struct ata_port *ap) { } +static inline int ata_acpi_on_devcfg (struct ata_device *adev) {return 0 ; }#endif /* libata-scsi.c */
include/linux/libata.h +2 -0
@@ -136,6 +136,8 @@ 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_ACPI_PENDING = (1 <<5 ),/* ACPI resume action pending */ +ATA_DFLAG_ACPI_FAILED = (1 <<6 ),/* ACPI on devcfg has failed */ ATA_DFLAG_CFG_MASK = (1 <<8 ) -1 , ATA_DFLAG_PIO = (1 << 8), /* device limited to PIO mode */
Signature · 1 line
1.5.0.3
PATCH 12/13 libata-acpi: remove redundant checks
In reply to Tejun HeoRemove remaining unnecessary feature and status checks.
--- drivers/ata/libata-acpi.c | 23 ----------------------- 1 files changed, 0 insertions(+), 23 deletions(-)
drivers/ata/libata-acpi.c +0 -23
@@ -134,22 +134,10 @@ static int do_drive_get_GTF(struct ata_device *adev, struct ata_acpi_gtf **gtf, output.length =ACPI_ALLOCATE_BUFFER ; output.pointer =NULL ;/* ACPI-CA sets this; save/free it later */ -if (!adev->acpi_handle) - goto out_free; -if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: ENTER: port#: %d\n" , __FUNCTION__, ap->port_no); -if (!ata_dev_enabled (adev) || (ap->flags &ATA_FLAG_DISABLED )) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG ,"%s: ERR: " -"ata_dev_present: %d, PORT_DISABLED: %lu\n" , - __FUNCTION__,ata_dev_enabled (adev), - ap->flags &ATA_FLAG_DISABLED ); - goto out_free; - } -/* _GTF has no input parameters */ status =acpi_evaluate_object (adev->acpi_handle,"_GTF" ,NULL , &output);if (ACPI_FAILURE (status)) {
@@ -339,21 +327,10 @@ static int ata_acpi_push_id(struct ata_device *adev)struct acpi_object_list input;union acpi_object in_params[1 ]; -if (!adev->acpi_handle) -return 0 ; -if (ata_msg_probe (ap))ata_dev_printk (adev,KERN_DEBUG ,"%s: ix = %d, port#: %d\n" , __FUNCTION__, adev->devno, ap->port_no); -/* Don't continue if not a SATA device. */ -if (!(ap->flags &ATA_FLAG_ACPI_SATA )) { -if (ata_msg_probe (ap)) -ata_dev_printk (adev,KERN_DEBUG , -"%s: Not a SATA device\n" , __FUNCTION__); -return 0 ; - } -/* Give the drive Identify data to the drive via the _SDD method */ /* _SDD: set up input parameters */ input.count =1 ;
Signature · 1 line
1.5.0.3
PATCH 13/13 libata-acpi: implement _GTM/_STM support
In reply to Tejun HeoImplement _GTM/_STM support. acpi_gtm is added to ata_port which stores _GTM parameters over suspend/resume cycle. A new hook ata_acpi_on_suspend() is responsible for storing _GTM parameters during suspend. _STM is executed in ata_acpi_on_resume(). With this change, invoking _GTF is safe on IDE hierarchy and acpi_sata check before _GTF is removed.
ata_acpi_gtm() and ata_acpi_stm() implementation is taken from Alan Cox's pata_acpi implementation. ata_acpi_gtm() is fixed such that the result parameter is not shifted by sizeof(union acpi_object).
--- drivers/ata/libata-acpi.c | 153 +++++++++++++++++++++++++++++++++++++++++++- drivers/ata/libata-eh.c | 8 ++- drivers/ata/libata.h | 2 + include/linux/libata.h | 13 ++++ 4 files changed, 171 insertions(+), 5 deletions(-)
drivers/ata/libata-acpi.c +149 -4
@@ -101,6 +101,108 @@ void ata_acpi_associate(struct ata_host *host) }/** +* ata_acpi_gtm - execute _GTM +* @ap: target ATA port +* @gtm: out parameter for _GTM result +* +* Evaluate _GTM and store the result in @gtm. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -ENOENT if _GTM doesn't exist, -errno on failure. +*/ +static int ata_acpi_gtm (const struct ata_port *ap,struct ata_acpi_gtm *gtm) +{ +struct acpi_buffer output = { .length =ACPI_ALLOCATE_BUFFER }; +union acpi_object *out_obj; +acpi_status status; +int rc =0 ; + + status =acpi_evaluate_object (ap->acpi_handle,"_GTM" ,NULL , &output); + + rc = -ENOENT ; +if (status ==AE_NOT_FOUND ) + goto out_free; + + rc = -EINVAL ; +if (ACPI_FAILURE (status)) { +ata_port_printk (ap,KERN_ERR , +"ACPI get timing mode failed (AE 0x%x)\n" , + status); + goto out_free; + } + + out_obj = output.pointer; +if (out_obj->type !=ACPI_TYPE_BUFFER ) { +ata_port_printk (ap,KERN_WARNING , +"_GTM returned unexpected object type 0x%x\n" , + out_obj->type); + + goto out_free; + } + +if (out_obj->buffer.length !=sizeof (struct ata_acpi_gtm )) { +ata_port_printk (ap,KERN_ERR , +"_GTM returned invalid length %d\n" , + out_obj->buffer.length); + goto out_free; + } + +memcpy (gtm, out_obj->buffer.pointer,sizeof (struct ata_acpi_gtm )); + rc =0 ; + out_free: +kfree (output.pointer); +return rc; +} + +/** +* ata_acpi_stm - execute _STM +* @ap: target ATA port +* @stm: timing parameter to _STM +* +* Evaluate _STM with timing parameter @stm. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -ENOENT if _STM doesn't exist, -errno on failure. +*/ +static int ata_acpi_stm (const struct ata_port *ap,struct ata_acpi_gtm *stm) +{ +acpi_status status; +struct acpi_object_list input; +union acpi_object in_params[3 ]; + + in_params[0 ].type =ACPI_TYPE_BUFFER ; + in_params[0 ].buffer.length =sizeof (struct ata_acpi_gtm ); + in_params[0 ].buffer.pointer = (u8 *)stm; +/* Buffers for id may need byteswapping ? */ + in_params[1 ].type =ACPI_TYPE_BUFFER ; + in_params[1 ].buffer.length =512 ; + in_params[1 ].buffer.pointer = (u8 *)ap->device[0 ].id; + in_params[2 ].type =ACPI_TYPE_BUFFER ; + in_params[2 ].buffer.length =512 ; + in_params[2 ].buffer.pointer = (u8 *)ap->device[1 ].id; + + input.count =3 ; + input.pointer = in_params; + + status =acpi_evaluate_object (ap->acpi_handle,"_STM" , &input,NULL ); + +if (status ==AE_NOT_FOUND ) +return -ENOENT ; +if (ACPI_FAILURE (status)) { +ata_port_printk (ap,KERN_ERR , +"ACPI set timing mode failed (status=0x%x)\n" , status); +return -EINVAL ; + } +return 0 ; +} + +/** * do_drive_get_GTF - get the drive bootup default taskfile settings * @adev: target ATA device * @gtf: output parameter for buffer containing _GTF taskfile arrays
@@ -354,6 +456,46 @@ static int ata_acpi_push_id(struct ata_device *adev) }/** +* ata_acpi_on_suspend - ATA ACPI hook called on suspend +* @ap: target ATA port +* +* This function is called when @ap is about to be suspended. All +* devices are already put to sleep but the port_suspend() callback +* hasn't been executed yet. Error return from this function aborts +* suspend. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ +int ata_acpi_on_suspend (struct ata_port *ap) +{ +unsigned long flags; +int rc; + +/* proceed iff per-port acpi_handle is valid */ +if (!ap->acpi_handle) +return 0 ; +BUG_ON (ap->flags &ATA_FLAG_ACPI_SATA ); + +/* store timing parameters */ + rc =ata_acpi_gtm (ap, &ap->acpi_gtm); + +spin_lock_irqsave (ap->lock, flags); +if (rc ==0 ) + ap->pflags |=ATA_PFLAG_GTM_VALID ; +else + ap->pflags &= ~ATA_PFLAG_GTM_VALID ; +spin_unlock_irqrestore (ap->lock, flags); + +if (rc == -ENOENT ) + rc =0 ; +return rc; +} + +/** * ata_acpi_on_resume - ATA ACPI hook called on resume * @ap: target ATA port *
@@ -367,6 +509,13 @@ void ata_acpi_on_resume(struct ata_port *ap) {int i; +if (ap->acpi_handle && (ap->pflags &ATA_PFLAG_GTM_VALID )) { +BUG_ON (ap->flags &ATA_FLAG_ACPI_SATA ); + +/* restore timing parameters */ +ata_acpi_stm (ap, &ap->acpi_gtm); + } +/* schedule _GTF */ for (i =0 ; i <ATA_MAX_DEVICES ; i++) ap->device[i].flags |=ATA_DFLAG_ACPI_PENDING ;
@@ -393,10 +542,6 @@ int ata_acpi_on_devcfg(struct ata_device *adev)int acpi_sata = ap->flags &ATA_FLAG_ACPI_SATA ;int rc; -/* XXX: _STM isn't implemented yet, skip if IDE for now */ -if (!acpi_sata) -return 0 ; -if (!adev->acpi_handle)return 0 ;
drivers/ata/libata-eh.c +7 -1
@@ -2335,19 +2335,25 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)WARN_ON (ap->pflags &ATA_PFLAG_SUSPENDED ); +/* tell ACPI we're suspending */ + rc =ata_acpi_on_suspend (ap); +if (rc) + goto out; +/* suspend */ ata_eh_freeze_port (ap);if (ap->ops->port_suspend) rc = ap->ops->port_suspend (ap, ap->pm_mesg); + out:/* report result */ spin_lock_irqsave (ap->lock, flags); ap->pflags &= ~ATA_PFLAG_PM_PENDING ;if (rc ==0 ) ap->pflags |=ATA_PFLAG_SUSPENDED ; -else +else if (ap->pflags &ATA_PFLAG_FROZEN )ata_port_schedule_eh (ap);if (ap->pm_result) {
drivers/ata/libata.h +2 -0
@@ -99,10 +99,12 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);/* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI extern void ata_acpi_associate (struct ata_host *host); +extern int ata_acpi_on_suspend (struct ata_port *ap);extern void ata_acpi_on_resume (struct ata_port *ap);extern int ata_acpi_on_devcfg (struct ata_device *adev);#else static inline void ata_acpi_associate (struct ata_host *host) { } +static inline int ata_acpi_on_suspend (struct ata_port *ap) {return 0 ; }static inline void ata_acpi_on_resume (struct ata_port *ap) { }static inline int ata_acpi_on_devcfg (struct ata_device *adev) {return 0 ; }#endif
include/linux/libata.h +13 -0
@@ -198,6 +198,7 @@ enum {ATA_PFLAG_FLUSH_PORT_TASK = (1 <<16 ),/* flush port task */ ATA_PFLAG_SUSPENDED = (1 <<17 ),/* port is suspended (power) */ ATA_PFLAG_PM_PENDING = (1 <<18 ),/* PM operation pending */ +ATA_PFLAG_GTM_VALID = (1 <<19 ),/* acpi_gtm data valid */ /* struct ata_queued_cmd flags */ ATA_QCFLAG_ACTIVE = (1 << 0), /* cmd not yet ack'd to scsi lyer */
@@ -512,6 +513,17 @@ struct ata_eh_context {unsigned int did_probe_mask; }; +struct ata_acpi_drive +{ +u32 pio; +u32 dma; +} __packed; + +struct ata_acpi_gtm { +struct ata_acpi_drive drive[2 ]; +u32 flags; +} __packed; +struct ata_port {struct Scsi_Host *scsi_host;/* our co-allocated scsi host */ const struct ata_port_operations *ops;
@@ -574,6 +586,7 @@ struct ata_port {#ifdef CONFIG_ATA_ACPI acpi_handle acpi_handle; +struct ata_acpi_gtm acpi_gtm;#endif u8 sector_buf[ATA_SECT_SIZE ];/* owned by EH */ };
Signature · 1 line
1.5.0.3
Re: [PATCH 01/13] ahci: consolidate common port flags
In reply to Tejun HeoQuoted 4 lines · On Mon, 23 Apr 2007 02:41:05 +0900 Tejun Heo <[email protected]> wrote:
Consolidate common port flags into
AHCI_FLAG_COMMON.Signed-off-by Tejun Heo <[email protected]>
Re: [PATCH 02/13] libata: separate ATA_EHI_DID_RESET into DID_SOFTRESET and DID_HARDRESET
In reply to Tejun HeoQuoted 6 lines · On Mon, 23 Apr 2007 02:41:05 +0900 Tejun Heo <[email protected]> wrote:
Separate
ATA_EHI_DID_RESETintoATA_EHI_DID_SOFTRESETandATA_EHI_DID_HARDRESET.ATA_EHI_DID_RESETis redefined as OR of the two flags. This patch doesn't introduce any behavior change. This will be used later to determine whether_SDDis necessary or not.Signed-off-by Tejun Heo <[email protected]>
Quoted 1 line
Alan Cox
NAKRe: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Tejun HeoQuoted 3 lines · On Mon, 23 Apr 2007 02:41:06 +0900 Tejun Heo <[email protected]> wrote:
Whether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA
NAK
I keep trying to point out that this is not true.
The ACPI interface to use can only be safely determined one way - and that is to see what methods the BIOS has attached to the device and use those.
Take the ACPI handle, go look for _GTF, _SDD etc and believe the firmware. Nothing else works.
Alan
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
Quoted 6 lines
The ACPI interface to use can only be safely determined one way - and that is to see what methods the BIOS has attached to the device and use those.
Take the ACPI handle, go look for
_GTF,_SDDetc and believe the firmware. Nothing else works.
Actually its even worse a mess than I thought 8(
We have to look at the BIOS method offered and we have to know if we changed the mode of the controller.
If the BIOS mode of an IDE controller is SFF and we flip it into AHCI mode then we can no longer use the BIOS provided methods because they are the wrong ones.
For the sane cases (where we don't frob the device) we can ask the BIOS what methods it has to guess what to use. If we change the mode of the controller we need to look for methods that match our change, and be aware we may have totally hosed the ACPI method support by changing the chip config.
Bletch, ACPI, its like openprom crossed with european law, a million times larger than needed, and full of holes.
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Alan CoxQuoted 9 lines · Alan Cox
Quoted 3 lines · On Mon, 23 Apr 2007 02:41:06 +0900 Tejun Heo <[email protected]> wrote:
Whether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA
NAK
Quoted 8 lines
I keep trying to point out that this is not true.
The ACPI interface to use can only be safely determined one way - and that is to see what methods the BIOS has attached to the device and use those.
Take the ACPI handle, go look for
_GTF,_SDDetc and believe the firmware. Nothing else works.
Actually, that's dangerous. For example, you must not do _STM/_GTM on ahci becuase _STM/_GTM access PCI config registers which must not be accessed in achi modes and some BIOSen supply the same _STM/_GTM nodes whether the controller is in ata_piix mode or ahci mode. Also, on ICH8, the association gets quite weird due to PCI device splitting.
The ACPI spec says the layout is dependent on controller interface and I
can see reasons why we need to follow that but not the other way
around. Do you have counter-examples?
Signature · 1 line
tejun
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Alan CoxQuoted 16 lines · Alan Cox
Quoted 6 lines
The ACPI interface to use can only be safely determined one way - and that is to see what methods the BIOS has attached to the device and use those.
Take the ACPI handle, go look for
_GTF,_SDDetc and believe the firmware. Nothing else works.Actually its even worse a mess than I thought 8(
We have to look at the BIOS method offered and we have to know if we changed the mode of the controller.
If the BIOS mode of an IDE controller is SFF and we flip it into AHCI mode then we can no longer use the BIOS provided methods because they are the wrong ones.
Quoted 8 lines
For the sane cases (where we don't frob the device) we can ask the BIOS what methods it has to guess what to use. If we change the mode of the controller we need to look for methods that match our change, and be aware we may have totally hosed the ACPI method support by changing the chip config.
Bletch, ACPI, its like openprom crossed with european law, a million times larger than needed, and full of holes.
Here are what I've seen till now.
* Many older boards only supply IDE style ACPI nodes and don't bother to switch them off when the controller is put into ahci mode (whether it's done by the BIOS itself or by the OS).
* Newer ones have both in their SSDTs. Probably selects one or the other according to BIOS setting. I haven't seen them in action and not too familiar with asl so dunno.
* Laptops which make heavy use of ATA ACPI features usually locks the controller into single programming mode and supply the matching ACPI hierarchy.
So, I think we'll be okay if we follow the spec here.
Signature · 1 line
tejun
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Tejun HeoQuoted 8 lines
Quoted 2 lines
Take the ACPI handle, go look for
_GTF,_SDDetc and believe the firmware. Nothing else works.Actually, that's dangerous. For example, you must not do _STM/_GTM on ahci becuase _STM/_GTM access PCI config registers which must not be accessed in achi modes and some BIOSen supply the same _STM/_GTM nodes whether the controller is in
ata_piixmode or ahci mode. Also, on ICH8, the association gets quite weird due to PCI device splitting.
You can end up with _SDD methods for SFF format controllers (or what we think of as SFF format controllers) when we don't have the knowledge to drive them in their full super-whizzo method (eg Marvell)
Quoted 3 lines
The ACPI spec says the layout is dependent on controller interface and I
can see reasons why we need to follow that but not the other way
around. Do you have counter-examples?
The first problem is the words "the spec".
Agreed that STM/GTM are not safe except when expected (and not always safe when they are) but that ought to be ok as we then set all the modes properly ourselves afterwards.
Quoted 2 lines
* after successfully executing
_GTFtaskfiles, IDENTIFY page is reloaded
Interesting question we should sort out: What is our identify page as supplied to the user meant to be ?
The old IDE one started off as the "identify data at boot" (which is useful) and mutated through a million "kind of boot but mangled" and "not boot" versions of the identify data, all of which are useless to userspace.
Having the "boot" data (for some definition of boot) is important in order to know things like the BIOS view of the disk geometry and HPA (eg for partitioning), having the current data is useless as its already available via SG_IO.
Alan
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Alan CoxQuoted 8 lines · Alan Cox
You can end up with
_SDDmethods for SFF format controllers (or what we think of as SFF format controllers) when we don't have the knowledge to drive them in their full super-whizzo method (eg Marvell)Quoted 3 lines
The ACPI spec says the layout is dependent on controller interface and I
can see reasons why we need to follow that but not the other way
around. Do you have counter-examples?
Quoted 5 lines
The first problem is the words "the spec".
Agreed that STM/GTM are not safe except when expected (and not always safe when they are) but that ought to be ok as we then set all the modes properly ourselves afterwards.
I agree that there are cases where we drive a controller in different way than how the BIOS configured it and as a result we can end up with unmatching ACPI nodes, but I think that it's safer to ignore unmatched ACPI nodes than to do what seems likely without really knowing what's going on.
There just isn't a generic way to determine which node matches which device. The first two ports might map to the first IDE channel or port 1 and 3 (ata_piix). Or each port might map to a separate IDE channel with slave spot disabled. Even if we support 'best effort' matching, it is something that only the LLD knows how it should be done and whether it's safe or not. IMHO, we're better off just ignoring ATA ACPI hierarchy if it doesn't match the expectation of the driver.
Signature · 1 line
tejun
Quoted 11 lines · Alan Cox
Quoted 2 lines
* after successfully executing
_GTFtaskfiles, IDENTIFY page is reloadedInteresting question we should sort out: What is our identify page as supplied to the user meant to be ?
The old IDE one started off as the "identify data at boot" (which is useful) and mutated through a million "kind of boot but mangled" and "not boot" versions of the identify data, all of which are useless to userspace.
Quoted 4 lines
Having the "boot" data (for some definition of boot) is important in order to know things like the BIOS view of the disk geometry and HPA (eg for partitioning), having the current data is useless as its already available via
SG_IO.
AFAIK, IDENTIFY using SG_IO is the way to obtain ID data. HDIO_GET_IDENTITY is only added for backward compatibility and as a sideway to obtain ATAPI IDENTIFY page while IDENTIFY_PACKET_DEVICE via SG_IO is broken. libata tries pretty hard to keep the cached IDENTIFY page up-to-date and I'm pretty sure it stays in sync with the drive most of the time. I think HDIO_GET_IDENTITY's meaning is something like "give me IDENTIFY page of the device as seen by the driver" and doesn't really matter as long as it can be used to get general idea about the device.
Signature · 1 line
tejun
Quoted 11 lines · Alan Cox
Quoted 2 lines
* after successfully executing
_GTFtaskfiles, IDENTIFY page is reloadedInteresting question we should sort out: What is our identify page as supplied to the user meant to be ?
The old IDE one started off as the "identify data at boot" (which is useful) and mutated through a million "kind of boot but mangled" and "not boot" versions of the identify data, all of which are useless to userspace.
Quoted 4 lines
Having the "boot" data (for some definition of boot) is important in order to know things like the BIOS view of the disk geometry and HPA (eg for partitioning), having the current data is useless as its already available via
SG_IO.
Agreed. Userspace can already fetch the real, current IDENTIFY data just by issuing the appropriate drive command (except for ATAPI, since Jeff has gone mute on the months old patch to fix it).
Boot-time IDENTIFY data was extremely useful for IDE development. It showed how the BIOS had set things up (or not).
Cheers
Quoted 5 lines · Tejun Heo
of the time. I think HDIO_GET_IDENTITY's meaning is something like "give me IDENTIFY page of the device as seen by the driver" and doesn't really matter as long as it can be used to get general idea about the device.
The correct/original meaning of that ioctl was "boot time IDENTIFY" data. As Alan said, it later got bastardized by various people at some point.
Cheers
Hi,
Quoted 9 lines · Mark Lord
Quoted 5 lines · Tejun Heo
of the time. I think HDIO_GET_IDENTITY's meaning is something like "give me IDENTIFY page of the device as seen by the driver" and doesn't really matter as long as it can be used to get general idea about the device.
The correct/original meaning of that ioctl was "boot time IDENTIFY" data. As Alan said, it later got bastardized by various people at some point.
It got de-bastardized later :) and currently HDIO_GET_IDENTITY seems to be returning "boot time IDENTIFY" with only two exceptions:
* if ID can't be read et all - fake geometry with user supplied values to make things work - this is OK (was added by Alan IIRC)
* id->dma_{1word,mword,ultra} (currently selected DMA transfer modes) are updated when the current transfer mode changes - this is of least importance than capacity/geometry data (since nowadays all speed tuning should be done by kernel anyway) and easy to fix once DMA rewrite patches from my tree get merged
Thanks,
Bart
Re: [PATCH 01/13] ahci: consolidate common port flags
In reply to Tejun HeoQuoted 6 lines · Tejun Heo
Consolidate common port flags into
AHCI_FLAG_COMMON.Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/ahci.c | 29 ++++++++++------------------- 1 files changed, 10 insertions(+), 19 deletions(-)
applied patches 1-2
Re: [PATCH 03/13] libata: separate out ata_dev_reread_id()
In reply to Tejun HeoQuoted 10 lines · Tejun Heo
Separate out
ata_dev_reread_id()fromata_dev_revalidate().ata_dev_reread_id()reads IDENTIFY page and determines whether the same device is still there.ata_dev_revalidate()reconfigures after reread completes. This will be used by ACPI update.Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-core.c | 47 ++++++++++++++++++++++++++++++++------------ drivers/ata/libata.h | 3 +- 2 files changed, 36 insertions(+), 14 deletions(-)
ACK, though I think you'll have to rediff due to a Mark Lord patch
Re: [PATCH 05/13] libata-acpi: s/CONFIG_SATA_ACPI/CONFIG_ATA_ACPI/
In reply to Tejun HeoQuoted 10 lines · Tejun Heo
ACPI applies to both SATA and PATA. Drop the 'S' from the config variable.
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/Kconfig | 26 +++++++++++++------------- drivers/ata/Makefile | 2 +- drivers/ata/libata.h | 2 +- include/linux/libata.h | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-)
ACK patches 4-5
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
In reply to Tejun HeoQuoted 7 lines · Tejun Heo
This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes.
* make libata-acpi functions take
ata_deviceinstead ofata_port+ device index
* s/atadev/adev/
* de-indent local variable declarations
I prefer 'dev' over 'adev', unless that makes the code in question more ambiguous.
Otherwise, ACK
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Tejun HeoQuoted 7 lines · Tejun Heo
Whether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA, or it supports slave device or not. This patch adds
ATA_FLAG_ACPI_SATAport flags which tells libata-acpi that the port needs SATA ACPI nodes, and sets the flag for ahci andsata_sil24.
Quoted 7 lines
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/ahci.c | 3 ++- drivers/ata/libata-acpi.c | 10 +++++----- drivers/ata/sata_sil24.c | 3 ++- include/linux/libata.h | 1 + 4 files changed, 10 insertions(+), 7 deletions(-)
I don't think the situation is as static as a compiled-in driver flag implies. And I'm not really convinced a driver flag is needed, or wanted.
If anything, the only flag we /may/ need could be a ATA_FLAG_NEVER_EVER_DO_ACPI_FOR_THIS_CONTROLLER.
Jeff
Re: [PATCH 08/13] libata-acpi: implement ata_acpi_associate()
In reply to Tejun HeoQuoted 13 lines · Tejun Heo
* Add
acpi_handletoata_hostandata_port. Renameata_device->obj_handleto ->acpi_handle and move it above such that it doesn't get cleared on reconfiguration.* Replace ACPI node association which
ata_acpi_associate()which is called once during host initialization. Unlike the previous implementation,ata_acpi_associate()usesATA_FLAG_ACPI_SATAto choose between IDE or SATA ACPI hierarchy and uses simple child look up instead of recursive walk to match the nodes. This is way safer and simpler. Please read the following message for more info.
Quoted 7 lines
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-acpi.c | 369 ++++++--------------------------------------- drivers/ata/libata-core.c | 3 + drivers/ata/libata.h | 2 + include/linux/libata.h | 13 +- 4 files changed, 63 insertions(+), 324 deletions(-)
largely dependent on previous patch's comments
Re: [PATCH 09/13] libata-acpi: clean up ata_acpi_exec_tfs()
In reply to Tejun HeoQuoted 16 lines · Tejun Heo
This patch cleans up
ata_acpi_exec_tfs()and its friends.* Rename
taskfile_arraytoata_acpi_gtfand make it__packedas it's used as argument to ACPI method, and use pointer toata_acpi_gtfand number of taskfiles to represent_GTFtaskfiles instead of a pointer casted into unsigned long and byte count. This makes argument re-checking indo_drive_set_taskfiles()unnecessary.* Pointer in void * not in unsigned long.
* Clean up
do_drive_get_GTF()error handling and makedo_drive_get_GTF()return number of taskfiles on success, 0 if_GTFdoesn't exist or doesn't contain valid ata. -errno on other errors.* Remove superflous check for
acpi->buffer.pointer.
Quoted 7 lines
* Update
taskfile_load_raw()such that printed messages look similar to the messages printed byata_eh_report().Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-acpi.c | 219 ++++++++++++++++++++++----------------------- 1 files changed, 107 insertions(+), 112 deletions(-)
ACK
As an aside, I hate the "do_" prefix on functions. It is utterly redundant.
Re: [PATCH 10/13] libata-acpi: miscellaneous cleanups
In reply to Tejun HeoQuoted 11 lines · Tejun Heo
* Add missing LOCKING: and RETURNS: to function comment.
* Don't conditionalize warning messages with
ata_msg_probe(). Print directly withKERN_WARNING.* Drop duplicate debug messages.
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-acpi.c | 51 ++++++++++++++++++++------------------------ 1 files changed, 23 insertions(+), 28 deletions(-)
ACK
Re: [PATCH 11/13] libata: reimplement ACPI invocation
In reply to Tejun Heopatches 11-13 seem sane
My general feeling is
* Alan's comments to most of the patches seemed to be on target
* I hate ACPI, and am terribly pleased you are working on this. Thanks. You will find you have much latitude in this area.
Re: [PATCH 03/13] libata: separate out ata_dev_reread_id()
In reply to Jeff GarzikQuoted 6 lines · Jeff Garzik
Quoted 5 lines · Tejun Heo
Separate out
ata_dev_reread_id()fromata_dev_revalidate().ata_dev_reread_id()reads IDENTIFY page and determines whether the same device is still there.ata_dev_revalidate()reconfigures after reread completes. This will be used by ACPI update.
Quoted 8 lines
Quoted 6 lines
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-core.c | 47 ++++++++++++++++++++++++++++++++------------ drivers/ata/libata.h | 3 +- 2 files changed, 36 insertions(+), 14 deletions(-)ACK, though I think you'll have to rediff due to a Mark Lord patch
No problem.
Signature · 1 line
tejun
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
In reply to Jeff GarzikQuoted 11 lines · Jeff Garzik
Quoted 7 lines · Tejun Heo
This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes.
* make libata-acpi functions take
ata_deviceinstead ofata_port+ device index
* s/atadev/adev/
* de-indent local variable declarationsI prefer 'dev' over 'adev', unless that makes the code in question more ambiguous.
Alan is preferring adev over dev and I thought that might be better in the spirit of 'ap'. I don't really care about the naming tho. Will convert to dev. It won't increase ambiguity.
Signature · 1 line
tejun
Re: [PATCH 07/13] libata-acpi: add ATA_FLAG_ACPI_SATA port flag
In reply to Jeff GarzikQuoted 16 lines · Jeff Garzik
Quoted 7 lines · Tejun Heo
Whether a controller needs IDE or SATA ACPI hierarchy is determined by the programming interface of the controller not by whether the controller is SATA or PATA, or it supports slave device or not. This patch adds
ATA_FLAG_ACPI_SATAport flags which tells libata-acpi that the port needs SATA ACPI nodes, and sets the flag for ahci andsata_sil24.Quoted 7 lines
Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/ahci.c | 3 ++- drivers/ata/libata-acpi.c | 10 +++++----- drivers/ata/sata_sil24.c | 3 ++- include/linux/libata.h | 1 + 4 files changed, 10 insertions(+), 7 deletions(-)
Quoted 5 lines
I don't think the situation is as static as a compiled-in driver flag implies. And I'm not really convinced a driver flag is needed, or wanted.
If anything, the only flag we /may/ need could be a
ATA_FLAG_NEVER_EVER_DO_ACPI_FOR_THIS_CONTROLLER.
Can you please elaborate a bit? As I wrote while talking with Alan, I really don't know how to do auto-matching. Personally, I don't think there is a way to do that safely but will be happy to implement it if someone can enlighten me. :-)
Signature · 1 line
tejun
Re: [PATCH 09/13] libata-acpi: clean up ata_acpi_exec_tfs()
In reply to Jeff GarzikQuoted 26 lines · Jeff Garzik
Quoted 16 lines · Tejun Heo
This patch cleans up
ata_acpi_exec_tfs()and its friends.* Rename
taskfile_arraytoata_acpi_gtfand make it__packedas it's used as argument to ACPI method, and use pointer toata_acpi_gtfand number of taskfiles to represent_GTFtaskfiles instead of a pointer casted into unsigned long and byte count. This makes argument re-checking indo_drive_set_taskfiles()unnecessary.* Pointer in void * not in unsigned long.
* Clean up
do_drive_get_GTF()error handling and makedo_drive_get_GTF()return number of taskfiles on success, 0 if_GTFdoesn't exist or doesn't contain valid ata. -errno on other errors.* Remove superflous check for
acpi->buffer.pointer.Quoted 8 lines
* Update
taskfile_load_raw()such that printed messages look similar to the messages printed byata_eh_report().Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/libata-acpi.c | 219 ++++++++++++++++++++++----------------------- 1 files changed, 107 insertions(+), 112 deletions(-)
Quoted 4 lines
ACK
As an aside, I hate the "
do_" prefix on functions. It is utterly redundant.
Okay, will drop.
Signature · 1 line
tejun
Re: [PATCH 06/13] libata-acpi: clean up parameters and misc stuff
In reply to Tejun HeoQuoted 9 lines · Tejun Heo
Quoted 8 lines · Jeff Garzik
Quoted 7 lines · Tejun Heo
This patch cleans up libata-acpi such that it looks similar to other libata files. This patch doesn't introuce any behavior changes.
* make libata-acpi functions take
ata_deviceinstead ofata_port+ device index
* s/atadev/adev/
* de-indent local variable declarations
Quoted 6 lines
Quoted 2 lines
I prefer 'dev' over 'adev', unless that makes the code in question more ambiguous.
Alan is preferring adev over dev and I thought that might be better in the spirit of 'ap'. I don't really care about the naming tho. Will convert to dev. It won't increase ambiguity.
Cool. You will see 'dev' used universally in the code I wrote. It also matches well with "ata_dev_" prefixes, which are a bit better than "ata_adev_" prefix if one were to apply the alternate policy.
Yes, I sometimes spend way too much time pay attention to trivialities :)
Jeff