| Message ID | 1484745601-4769-10-git-send-email-bgolaszewski@baylibre.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello, Bartosz. On Wed, Jan 18, 2017 at 02:19:57PM +0100, Bartosz Golaszewski wrote: > We need a way to retrieve the information about the online state of > the link in the ahci-da850 driver. > > Create a new function: ahci_do_hardreset() which is called from > ahci_hardreset() for backwards compatibility, but has an additional > argument: 'online' - which can be used to check if the link is online > after this function returns. Please just add @online to ahci_hardreset() and update the callers. Other than that, the sata changes look good to me. Thanks.
2017-01-18 19:28 GMT+01:00 Tejun Heo <tj@kernel.org>: > Hello, Bartosz. > > On Wed, Jan 18, 2017 at 02:19:57PM +0100, Bartosz Golaszewski wrote: >> We need a way to retrieve the information about the online state of >> the link in the ahci-da850 driver. >> >> Create a new function: ahci_do_hardreset() which is called from >> ahci_hardreset() for backwards compatibility, but has an additional >> argument: 'online' - which can be used to check if the link is online >> after this function returns. > > Please just add @online to ahci_hardreset() and update the callers. > Other than that, the sata changes look good to me. > Are you sure? There are 23 places in drivers/ata/ where the .hardreset callback is assigned. I'd prefer not to change the drivers I can't test. Besides all other **reset callbacks take three arguments - should we really only change one of them for a single driver's needs? Thanks, Bartosz -- 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 Thu, Jan 19, 2017 at 11:55:24AM +0100, Bartosz Golaszewski wrote: > 2017-01-18 19:28 GMT+01:00 Tejun Heo <tj@kernel.org>: > > Hello, Bartosz. > > > > On Wed, Jan 18, 2017 at 02:19:57PM +0100, Bartosz Golaszewski wrote: > >> We need a way to retrieve the information about the online state of > >> the link in the ahci-da850 driver. > >> > >> Create a new function: ahci_do_hardreset() which is called from > >> ahci_hardreset() for backwards compatibility, but has an additional > >> argument: 'online' - which can be used to check if the link is online > >> after this function returns. > > > > Please just add @online to ahci_hardreset() and update the callers. > > Other than that, the sata changes look good to me. > > > > Are you sure? There are 23 places in drivers/ata/ where the .hardreset > callback is assigned. I'd prefer not to change the drivers I can't > test. Besides all other **reset callbacks take three arguments - > should we really only change one of them for a single driver's needs? Ah, didn't realize this was the callback, sorry. What you did is perfect. Please disregard my comment. Thanks.
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h index 0cc08f8..5db6ab2 100644 --- a/drivers/ata/ahci.h +++ b/drivers/ata/ahci.h @@ -398,6 +398,9 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class, int pmp, unsigned long deadline, int (*check_ready)(struct ata_link *link)); +int ahci_do_hardreset(struct ata_link *link, unsigned int *class, + unsigned long deadline, bool *online); + unsigned int ahci_qc_issue(struct ata_queued_cmd *qc); int ahci_stop_engine(struct ata_port *ap); void ahci_start_fis_rx(struct ata_port *ap); diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c index ee7db31..3159f9e 100644 --- a/drivers/ata/libahci.c +++ b/drivers/ata/libahci.c @@ -1519,8 +1519,8 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class, return rc; } -static int ahci_hardreset(struct ata_link *link, unsigned int *class, - unsigned long deadline) +int ahci_do_hardreset(struct ata_link *link, unsigned int *class, + unsigned long deadline, bool *online) { const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context); struct ata_port *ap = link->ap; @@ -1528,7 +1528,6 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class, struct ahci_host_priv *hpriv = ap->host->private_data; u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG; struct ata_taskfile tf; - bool online; int rc; DPRINTK("ENTER\n"); @@ -1540,17 +1539,26 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class, tf.command = ATA_BUSY; ata_tf_to_fis(&tf, 0, 0, d2h_fis); - rc = sata_link_hardreset(link, timing, deadline, &online, + rc = sata_link_hardreset(link, timing, deadline, online, ahci_check_ready); hpriv->start_engine(ap); - if (online) + if (*online) *class = ahci_dev_classify(ap); DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class); return rc; } +EXPORT_SYMBOL_GPL(ahci_do_hardreset); + +static int ahci_hardreset(struct ata_link *link, unsigned int *class, + unsigned long deadline) +{ + bool online; + + return ahci_do_hardreset(link, class, deadline, &online); +} static void ahci_postreset(struct ata_link *link, unsigned int *class) {
We need a way to retrieve the information about the online state of the link in the ahci-da850 driver. Create a new function: ahci_do_hardreset() which is called from ahci_hardreset() for backwards compatibility, but has an additional argument: 'online' - which can be used to check if the link is online after this function returns. The new routine will be used in the ahci-da850 driver to avoid code duplication when implementing a workaround for tha da850 SATA controller quirk/instability. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- drivers/ata/ahci.h | 3 +++ drivers/ata/libahci.c | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-)