[1/3] ata: Allow having a port recovery callback

Message ID 1515539097-26742-2-git-send-email-f.fainelli@gmail.com
State Not Applicable
Delegated to: David Miller
Headers show
Series
  • ata: ahci_brcm: Recover from failures to identify devices
Related show

Commit Message

Florian Fainelli Jan. 9, 2018, 11:04 p.m.
Some AHCI controllers may need to recovery from a failure to identify a drive,
such as the Broadcom AHCI controller, make that possible at both the
libata-core.c and AHCI controller level such that we can provide such an error
recovery callback.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/ata/libata-core.c | 2 ++
 include/linux/libata.h    | 1 +
 2 files changed, 3 insertions(+)

Comments

Tejun Heo Jan. 10, 2018, 2:25 p.m. | #1
Hello,

On Tue, Jan 09, 2018 at 03:04:55PM -0800, Florian Fainelli wrote:
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 3c09122bf038..921c2813af07 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2045,6 +2045,8 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
>  	if (ata_msg_warn(ap))
>  		ata_dev_warn(dev, "failed to IDENTIFY (%s, err_mask=0x%x)\n",
>  			     reason, err_mask);
> +	if (ap->host->ops->port_recovery)
> +		ap->host->ops->port_recovery(ap);
>  	return rc;
>  }

This is a really weird spot to add a callback named port_recovery().
Can't the affected driver simply implement its own
ata_port_operations->read_id() operation which does the recovery if
necessary?

Thanks.
Sergei Shtylyov Jan. 10, 2018, 4:02 p.m. | #2
Hello!

On 01/10/2018 02:04 AM, Florian Fainelli wrote:

> Some AHCI controllers may need to recovery from a failure to identify a drive,

    Recover.

> such as the Broadcom AHCI controller, make that possible at both the
> libata-core.c and AHCI controller level such that we can provide such an error
> recovery callback.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

MBR, Sergei
--
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
Florian Fainelli Jan. 10, 2018, 5:53 p.m. | #3
On 01/10/2018 06:25 AM, Tejun Heo wrote:
> Hello,
> 
> On Tue, Jan 09, 2018 at 03:04:55PM -0800, Florian Fainelli wrote:
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 3c09122bf038..921c2813af07 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -2045,6 +2045,8 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
>>  	if (ata_msg_warn(ap))
>>  		ata_dev_warn(dev, "failed to IDENTIFY (%s, err_mask=0x%x)\n",
>>  			     reason, err_mask);
>> +	if (ap->host->ops->port_recovery)
>> +		ap->host->ops->port_recovery(ap);
>>  	return rc;
>>  }
> 
> This is a really weird spot to add a callback named port_recovery().
> Can't the affected driver simply implement its own
> ata_port_operations->read_id() operation which does the recovery if
> necessary?

I did not consider that, but this is actually a great idea, thanks for
the suggestion! I will respin with that being done.

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3c09122bf038..921c2813af07 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2045,6 +2045,8 @@  int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 	if (ata_msg_warn(ap))
 		ata_dev_warn(dev, "failed to IDENTIFY (%s, err_mask=0x%x)\n",
 			     reason, err_mask);
+	if (ap->host->ops->port_recovery)
+		ap->host->ops->port_recovery(ap);
 	return rc;
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ed9826b21c5e..23646a557ab2 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -956,6 +956,7 @@  struct ata_port_operations {
 	int  (*port_resume)(struct ata_port *ap);
 	int  (*port_start)(struct ata_port *ap);
 	void (*port_stop)(struct ata_port *ap);
+	void (*port_recovery)(struct ata_port *ap);
 	void (*host_stop)(struct ata_host *host);
 
 #ifdef CONFIG_ATA_SFF