Message ID | 1340000766-129148-3-git-send-email-ming.m.lin@intel.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <ming.m.lin@intel.com> wrote: > From: Matthew Garrett <mjg@redhat.com> > > Associate the ACPI device tree and libata devices. > This patch uses the generic ACPI glue framework to do so. > > Signed-off-by: Matthew Garrett <mjg@redhat.com> > Signed-off-by: Holger Macht <holger@homac.de> > Signed-off-by: Lin Ming <ming.m.lin@intel.com> > --- > > Changelog: > > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu) > - rename is_pci_ata to compat_pci_ata (Alan Cox) > > drivers/acpi/glue.c | 4 +- > drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/ata/libata-core.c | 3 ++ > drivers/ata/libata.h | 4 ++ > 4 files changed, 134 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c > index 1564e09..18d6812 100644 > --- a/drivers/acpi/glue.c > +++ b/drivers/acpi/glue.c [..] > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle) > +{ > + struct Scsi_Host *shost = dev_to_shost(dev); > + struct ata_port *ap = ata_shost_to_port(shost); > + > + if (ap->flags & ATA_FLAG_ACPI_SATA) > + return -ENODEV; > + > + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no); > + > + if (!*handle) > + return -ENODEV; > + > + return 0; > +} I think this patch should be squashed with patch 4, right? That is if we keep the hard-coded ->parent chasing... > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle) > +{ > + unsigned int host, channel, id, lun; > + > + if (sscanf(dev_name(dev), "host%u", &host) == 1) { > + if (!compat_pci_ata(dev->parent)) > + return -ENODEV; > + > + return ata_acpi_bind_host(dev, host, handle); > + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d", > + &host, &channel, &id, &lun) == 4) { > + if (!compat_pci_ata(dev->parent->parent->parent)) > + return -ENODEV; ...this looks like it should be using a dev_to_ata_port() helper which like dev_to_shost() skips the need to remember just how many parents until we get back to our ata_port, scsi_host, etc. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2012-06-20 at 16:50 -0700, Dan Williams wrote: > On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <ming.m.lin@intel.com> wrote: > > From: Matthew Garrett <mjg@redhat.com> > > > > Associate the ACPI device tree and libata devices. > > This patch uses the generic ACPI glue framework to do so. > > > > Signed-off-by: Matthew Garrett <mjg@redhat.com> > > Signed-off-by: Holger Macht <holger@homac.de> > > Signed-off-by: Lin Ming <ming.m.lin@intel.com> > > --- > > > > Changelog: > > > > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu) > > - rename is_pci_ata to compat_pci_ata (Alan Cox) > > > > drivers/acpi/glue.c | 4 +- > > drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++ > > drivers/ata/libata-core.c | 3 ++ > > drivers/ata/libata.h | 4 ++ > > 4 files changed, 134 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c > > index 1564e09..18d6812 100644 > > --- a/drivers/acpi/glue.c > > +++ b/drivers/acpi/glue.c > [..] > > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle) > > +{ > > + struct Scsi_Host *shost = dev_to_shost(dev); > > + struct ata_port *ap = ata_shost_to_port(shost); > > + > > + if (ap->flags & ATA_FLAG_ACPI_SATA) > > + return -ENODEV; > > + > > + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no); > > + > > + if (!*handle) > > + return -ENODEV; > > + > > + return 0; > > +} > > I think this patch should be squashed with patch 4, right? That is if > we keep the hard-coded ->parent chasing... Right. Will merge patch 4. > > > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle) > > +{ > > + unsigned int host, channel, id, lun; > > + > > + if (sscanf(dev_name(dev), "host%u", &host) == 1) { > > + if (!compat_pci_ata(dev->parent)) > > + return -ENODEV; > > + > > + return ata_acpi_bind_host(dev, host, handle); > > + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d", > > + &host, &channel, &id, &lun) == 4) { > > + if (!compat_pci_ata(dev->parent->parent->parent)) > > + return -ENODEV; > > ...this looks like it should be using a dev_to_ata_port() helper which > like dev_to_shost() skips the need to remember just how many parents > until we get back to our ata_port, scsi_host, etc. Oh, no. compat_pci_ata checks whether the controller(rather than shost or ata port) is pci SATA/IDE controller. As in patch 4: compat_pci_ata(dev->parent->parent->parent->parent) The "dev" is ata port, and dev->parent->parent->parent->parent points to the controller dev. This looks ugly. How about add two helpers? shost_to_controller(dev) and ata_port_to_controller(dev) Thanks, Lin Ming > > -- > Dan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 20, 2012 at 6:26 PM, Lin Ming <ming.m.lin@intel.com> wrote: > On Wed, 2012-06-20 at 16:50 -0700, Dan Williams wrote: >> On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <ming.m.lin@intel.com> wrote: >> > From: Matthew Garrett <mjg@redhat.com> >> > >> > Associate the ACPI device tree and libata devices. >> > This patch uses the generic ACPI glue framework to do so. >> > >> > Signed-off-by: Matthew Garrett <mjg@redhat.com> >> > Signed-off-by: Holger Macht <holger@homac.de> >> > Signed-off-by: Lin Ming <ming.m.lin@intel.com> >> > --- >> > >> > Changelog: >> > >> > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu) >> > - rename is_pci_ata to compat_pci_ata (Alan Cox) >> > >> > drivers/acpi/glue.c | 4 +- >> > drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++ >> > drivers/ata/libata-core.c | 3 ++ >> > drivers/ata/libata.h | 4 ++ >> > 4 files changed, 134 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c >> > index 1564e09..18d6812 100644 >> > --- a/drivers/acpi/glue.c >> > +++ b/drivers/acpi/glue.c >> [..] >> > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle) >> > +{ >> > + struct Scsi_Host *shost = dev_to_shost(dev); >> > + struct ata_port *ap = ata_shost_to_port(shost); >> > + >> > + if (ap->flags & ATA_FLAG_ACPI_SATA) >> > + return -ENODEV; >> > + >> > + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no); >> > + >> > + if (!*handle) >> > + return -ENODEV; >> > + >> > + return 0; >> > +} >> >> I think this patch should be squashed with patch 4, right? That is if >> we keep the hard-coded ->parent chasing... > > Right. Will merge patch 4. > >> >> > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle) >> > +{ >> > + unsigned int host, channel, id, lun; >> > + >> > + if (sscanf(dev_name(dev), "host%u", &host) == 1) { >> > + if (!compat_pci_ata(dev->parent)) >> > + return -ENODEV; >> > + >> > + return ata_acpi_bind_host(dev, host, handle); >> > + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d", >> > + &host, &channel, &id, &lun) == 4) { >> > + if (!compat_pci_ata(dev->parent->parent->parent)) >> > + return -ENODEV; >> >> ...this looks like it should be using a dev_to_ata_port() helper which >> like dev_to_shost() skips the need to remember just how many parents >> until we get back to our ata_port, scsi_host, etc. > > Oh, no. > > compat_pci_ata checks whether the controller(rather than shost or ata > port) is pci SATA/IDE controller. > > As in patch 4: > compat_pci_ata(dev->parent->parent->parent->parent) > > The "dev" is ata port, and dev->parent->parent->parent->parent points to > the controller dev. > > This looks ugly. How about add two helpers? > shost_to_controller(dev) and ata_port_to_controller(dev) > What about something like the following (untested)? Makes it mostly independent of the topology, gives some type safety, and drops some redundant work finding the ata_port. static int ata_acpi_find_device(struct device *dev, acpi_handle *handle) { struct ata_port *ap = dev_to_ata_port(dev); if (!ap) return -ENODEV; if (!compat_pci_ata(ap)) return -ENODEV; if (scsi_is_host_device(dev)) return ata_acpi_bind_host(ap, handle); else if (scsi_is_sdev_device(dev)) { struct scsi_device *sdev = to_scsi_device(dev); return ata_acpi_bind_device(ap, sdev->channel, sdev->id, handle); } else return -ENODEV; } -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c index 1564e09..18d6812 100644 --- a/drivers/acpi/glue.c +++ b/drivers/acpi/glue.c @@ -39,6 +39,7 @@ int register_acpi_bus_type(struct acpi_bus_type *type) } return -ENODEV; } +EXPORT_SYMBOL(register_acpi_bus_type); int unregister_acpi_bus_type(struct acpi_bus_type *type) { @@ -54,6 +55,7 @@ int unregister_acpi_bus_type(struct acpi_bus_type *type) } return -ENODEV; } +EXPORT_SYMBOL(unregister_acpi_bus_type); static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type) { @@ -69,7 +71,6 @@ static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type) up_read(&bus_type_sem); return ret; } -EXPORT_SYMBOL_GPL(register_acpi_bus_type); static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle) { @@ -86,7 +87,6 @@ static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle) up_read(&bus_type_sem); return ret; } -EXPORT_SYMBOL_GPL(unregister_acpi_bus_type); /* Get device's handler per its address under its parent */ struct acpi_find_child { diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index bb7c5f1..ae93593 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -47,6 +47,39 @@ static void ata_acpi_clear_gtf(struct ata_device *dev) dev->gtf_cache = NULL; } +static acpi_handle ap_acpi_handle(struct ata_port *ap) +{ + if (ap->flags & ATA_FLAG_ACPI_SATA) + return NULL; + + /* + * If acpi bind operation has already happened, we can get the handle + * for the port by checking the corresponding scsi_host device's + * firmware node, otherwise we will need to find out the handle from + * its parent's acpi node. + */ + if (ap->scsi_host) + return DEVICE_ACPI_HANDLE(&ap->scsi_host->shost_gendev); + else + return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), + ap->port_no); +} + +static acpi_handle dev_acpi_handle(struct ata_device *dev) +{ + acpi_integer adr; + struct ata_port *ap = dev->link->ap; + + if (ap->flags & ATA_FLAG_ACPI_SATA) { + if (!sata_pmp_attached(ap)) + adr = SATA_ADR(ap->port_no, NO_PORT_MULT); + else + adr = SATA_ADR(ap->port_no, dev->link->pmp); + return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), adr); + } else + return acpi_get_child(ap_acpi_handle(ap), dev->devno); +} + /** * ata_acpi_associate_sata_port - associate SATA port with ACPI objects * @ap: target SATA port @@ -1018,3 +1051,95 @@ void ata_acpi_on_disable(struct ata_device *dev) { ata_acpi_clear_gtf(dev); } + +static int compat_pci_ata(struct device *dev) +{ + struct pci_dev *pdev; + + if (!is_pci_dev(dev)) + return 0; + + pdev = to_pci_dev(dev); + + if ((pdev->class >> 8) != PCI_CLASS_STORAGE_SATA && + (pdev->class >> 8) != PCI_CLASS_STORAGE_IDE) + return 0; + + return 1; +} + +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle) +{ + struct Scsi_Host *shost = dev_to_shost(dev); + struct ata_port *ap = ata_shost_to_port(shost); + + if (ap->flags & ATA_FLAG_ACPI_SATA) + return -ENODEV; + + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no); + + if (!*handle) + return -ENODEV; + + return 0; +} + +static int ata_acpi_bind_device(struct device *dev, int channel, int id, + acpi_handle *handle) +{ + struct device *host = dev->parent->parent; + struct Scsi_Host *shost = dev_to_shost(host); + struct ata_port *ap = ata_shost_to_port(shost); + struct ata_device *ata_dev; + + if (ap->flags & ATA_FLAG_ACPI_SATA) + ata_dev = &ap->link.device[channel]; + else + ata_dev = &ap->link.device[id]; + + *handle = dev_acpi_handle(ata_dev); + + if (!*handle) + return -ENODEV; + + return 0; +} + +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle) +{ + unsigned int host, channel, id, lun; + + if (sscanf(dev_name(dev), "host%u", &host) == 1) { + if (!compat_pci_ata(dev->parent)) + return -ENODEV; + + return ata_acpi_bind_host(dev, host, handle); + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d", + &host, &channel, &id, &lun) == 4) { + if (!compat_pci_ata(dev->parent->parent->parent)) + return -ENODEV; + + return ata_acpi_bind_device(dev, channel, id, handle); + } else + return -ENODEV; +} + +static int ata_acpi_find_dummy(struct device *dev, acpi_handle *handle) +{ + return -ENODEV; +} + +static struct acpi_bus_type ata_acpi_bus = { + .find_bridge = ata_acpi_find_dummy, + .find_device = ata_acpi_find_device, +}; + +int ata_acpi_register(void) +{ + return scsi_register_acpi_bus_type(&ata_acpi_bus); +} + +void ata_acpi_unregister(void) +{ + scsi_unregister_acpi_bus_type(&ata_acpi_bus); +} diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index cece3a4..5628bf6 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6513,6 +6513,8 @@ static int __init ata_init(void) ata_parse_force_param(); + ata_acpi_register(); + rc = ata_sff_init(); if (rc) { kfree(ata_force_tbl); @@ -6539,6 +6541,7 @@ static void __exit ata_exit(void) ata_release_transport(ata_scsi_transport_template); libata_transport_exit(); ata_sff_exit(); + ata_acpi_unregister(); kfree(ata_force_tbl); } diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 9d0fd0b..3df3362 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -119,6 +119,8 @@ extern void ata_acpi_on_resume(struct ata_port *ap); extern int ata_acpi_on_devcfg(struct ata_device *dev); extern void ata_acpi_on_disable(struct ata_device *dev); extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state); +extern int ata_acpi_register(void); +extern void ata_acpi_unregister(void); #else static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { } static inline void ata_acpi_associate(struct ata_host *host) { } @@ -129,6 +131,8 @@ static inline int ata_acpi_on_devcfg(struct ata_device *dev) { return 0; } static inline void ata_acpi_on_disable(struct ata_device *dev) { } static inline void ata_acpi_set_state(struct ata_port *ap, pm_message_t state) { } +static inline int ata_acpi_register(void) { return 0; } +static void ata_acpi_unregister(void) { } #endif /* libata-scsi.c */