| Message ID | 20240206211352.1664816-3-cassel@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | drop low power policy board type | expand |
Hi everbody, On 2024-02-06 22:13:43+0000, Niklas Cassel wrote: > A hotplug capable port is an external port, so mark it as such. > > We even say this ourselves in libata-scsi.c: > /* set scsi removable (RMB) bit per ata bit, or if the > * AHCI port says it's external (Hotplug-capable, eSATA). > */ > > This also matches the terminology used in AHCI 1.3.1 > (the keyword to search for is "externally accessible"). > > Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > Signed-off-by: Niklas Cassel <cassel@kernel.org> > --- > drivers/ata/ahci.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > index aa58ce615e79..4d3ec6d15ad1 100644 > --- a/drivers/ata/ahci.c > +++ b/drivers/ata/ahci.c > @@ -1648,9 +1648,10 @@ static void ahci_mark_external_port(struct ata_port *ap) > void __iomem *port_mmio = ahci_port_base(ap); > u32 tmp; > > - /* mark esata ports */ > + /* mark external ports (hotplug-capable, eSATA) */ > tmp = readl(port_mmio + PORT_CMD); > - if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) > + if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) || > + (tmp & PORT_CMD_HPCP)) > ap->pflags |= ATA_PFLAG_EXTERNAL; > } This seems to introduce a userspace regression. GNOME/udisks are now automounting internal disks, which they didn't before. See [0], [1], [2] ATA_PFLAG_EXTERNAL is translated into GENHD_FL_REMOVABLE. (Through ata_scsiop_inq_std(), scsi_add_lun(), sd_probe()) But GENHD_FL_REMOVABLE is not meant for hotpluggable devices but for media-changable devices (See its description in include/linux/blkdev.h). To indicate hotplug, dev_set_removable() is to be used. (Both end up in "removable" sysfs attributes, but these have different semantics...) #regzbot introduced: 45b96d65ec68f625ad26ee16d2f556e29f715005 [0] https://bbs.archlinux.org/viewtopic.php?id=295958 [1] https://github.com/storaged-project/udisks/issues/1282 [2] https://github.com/util-linux/util-linux/issues/3088
On 6/13/24 15:34, Thomas Weißschuh wrote: > Hi everbody, > > On 2024-02-06 22:13:43+0000, Niklas Cassel wrote: >> A hotplug capable port is an external port, so mark it as such. >> >> We even say this ourselves in libata-scsi.c: >> /* set scsi removable (RMB) bit per ata bit, or if the >> * AHCI port says it's external (Hotplug-capable, eSATA). >> */ >> >> This also matches the terminology used in AHCI 1.3.1 >> (the keyword to search for is "externally accessible"). >> >> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> >> Signed-off-by: Niklas Cassel <cassel@kernel.org> >> --- >> drivers/ata/ahci.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c >> index aa58ce615e79..4d3ec6d15ad1 100644 >> --- a/drivers/ata/ahci.c >> +++ b/drivers/ata/ahci.c >> @@ -1648,9 +1648,10 @@ static void ahci_mark_external_port(struct ata_port *ap) >> void __iomem *port_mmio = ahci_port_base(ap); >> u32 tmp; >> >> - /* mark esata ports */ >> + /* mark external ports (hotplug-capable, eSATA) */ >> tmp = readl(port_mmio + PORT_CMD); >> - if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) >> + if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) || >> + (tmp & PORT_CMD_HPCP)) >> ap->pflags |= ATA_PFLAG_EXTERNAL; >> } > > This seems to introduce a userspace regression. > > GNOME/udisks are now automounting internal disks, which they didn't before. > See [0], [1], [2] > > ATA_PFLAG_EXTERNAL is translated into GENHD_FL_REMOVABLE. > (Through ata_scsiop_inq_std(), scsi_add_lun(), sd_probe()) > > But GENHD_FL_REMOVABLE is not meant for hotpluggable devices but for > media-changable devices (See its description in include/linux/blkdev.h). > > To indicate hotplug, dev_set_removable() is to be used. > > (Both end up in "removable" sysfs attributes, but these have different > semantics...) This should take care of the issue. diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 37ded3875ea3..170ed47ef74a 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1912,11 +1912,8 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf) 2 }; - /* set scsi removable (RMB) bit per ata bit, or if the - * AHCI port says it's external (Hotplug-capable, eSATA). - */ - if (ata_id_removable(args->id) || - (args->dev->link->ap->pflags & ATA_PFLAG_EXTERNAL)) + /* Set scsi removable (RMB) bit per ata bit. */ + if (ata_id_removable(args->id)) hdr[1] |= (1 << 7); if (args->dev->class == ATA_DEV_ZAC) { BUT, need to check what SAT & SATA-IO have to say about this. > > #regzbot introduced: 45b96d65ec68f625ad26ee16d2f556e29f715005 > > [0] https://bbs.archlinux.org/viewtopic.php?id=295958 > [1] https://github.com/storaged-project/udisks/issues/1282 > [2] https://github.com/util-linux/util-linux/issues/3088 >
+Cc LKML for people to find it more easily. On 2024-06-13 17:29:31+0000, Damien Le Moal wrote: > On 6/13/24 15:34, Thomas Weißschuh wrote: > > Hi everbody, > > > > On 2024-02-06 22:13:43+0000, Niklas Cassel wrote: > >> A hotplug capable port is an external port, so mark it as such. > >> > >> We even say this ourselves in libata-scsi.c: > >> /* set scsi removable (RMB) bit per ata bit, or if the > >> * AHCI port says it's external (Hotplug-capable, eSATA). > >> */ > >> > >> This also matches the terminology used in AHCI 1.3.1 > >> (the keyword to search for is "externally accessible"). > >> > >> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > >> Signed-off-by: Niklas Cassel <cassel@kernel.org> > >> --- > >> drivers/ata/ahci.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > >> index aa58ce615e79..4d3ec6d15ad1 100644 > >> --- a/drivers/ata/ahci.c > >> +++ b/drivers/ata/ahci.c > >> @@ -1648,9 +1648,10 @@ static void ahci_mark_external_port(struct ata_port *ap) > >> void __iomem *port_mmio = ahci_port_base(ap); > >> u32 tmp; > >> > >> - /* mark esata ports */ > >> + /* mark external ports (hotplug-capable, eSATA) */ > >> tmp = readl(port_mmio + PORT_CMD); > >> - if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) > >> + if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) || > >> + (tmp & PORT_CMD_HPCP)) > >> ap->pflags |= ATA_PFLAG_EXTERNAL; > >> } > > > > This seems to introduce a userspace regression. > > > > GNOME/udisks are now automounting internal disks, which they didn't before. > > See [0], [1], [2] > > > > ATA_PFLAG_EXTERNAL is translated into GENHD_FL_REMOVABLE. > > (Through ata_scsiop_inq_std(), scsi_add_lun(), sd_probe()) > > > > But GENHD_FL_REMOVABLE is not meant for hotpluggable devices but for > > media-changable devices (See its description in include/linux/blkdev.h). > > > > To indicate hotplug, dev_set_removable() is to be used. > > > > (Both end up in "removable" sysfs attributes, but these have different > > semantics...) > > This should take care of the issue. > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 37ded3875ea3..170ed47ef74a 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -1912,11 +1912,8 @@ static unsigned int ata_scsiop_inq_std(struct > ata_scsi_args *args, u8 *rbuf) > 2 > }; > > - /* set scsi removable (RMB) bit per ata bit, or if the > - * AHCI port says it's external (Hotplug-capable, eSATA). > - */ > - if (ata_id_removable(args->id) || > - (args->dev->link->ap->pflags & ATA_PFLAG_EXTERNAL)) > + /* Set scsi removable (RMB) bit per ata bit. */ > + if (ata_id_removable(args->id)) > hdr[1] |= (1 << 7); > > if (args->dev->class == ATA_DEV_ZAC) { Thanks, looks good. Tested-by: Thomas Weißschuh <linux@weissschuh.net> > BUT, need to check what SAT & SATA-IO have to say about this. Who takes care of this? > > #regzbot introduced: 45b96d65ec68f625ad26ee16d2f556e29f715005 > > > > [0] https://bbs.archlinux.org/viewtopic.php?id=295958 > > [1] https://github.com/storaged-project/udisks/issues/1282 > > [2] https://github.com/util-linux/util-linux/issues/3088
On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > On 6/13/24 15:34, Thomas Weißschuh wrote: > > Hi everbody, > > > > On 2024-02-06 22:13:43+0000, Niklas Cassel wrote: > >> A hotplug capable port is an external port, so mark it as such. > >> > >> We even say this ourselves in libata-scsi.c: > >> /* set scsi removable (RMB) bit per ata bit, or if the > >> * AHCI port says it's external (Hotplug-capable, eSATA). > >> */ > >> > >> This also matches the terminology used in AHCI 1.3.1 > >> (the keyword to search for is "externally accessible"). > >> > >> Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > >> Signed-off-by: Niklas Cassel <cassel@kernel.org> > >> --- > >> drivers/ata/ahci.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c > >> index aa58ce615e79..4d3ec6d15ad1 100644 > >> --- a/drivers/ata/ahci.c > >> +++ b/drivers/ata/ahci.c > >> @@ -1648,9 +1648,10 @@ static void ahci_mark_external_port(struct ata_port *ap) > >> void __iomem *port_mmio = ahci_port_base(ap); > >> u32 tmp; > >> > >> - /* mark esata ports */ > >> + /* mark external ports (hotplug-capable, eSATA) */ > >> tmp = readl(port_mmio + PORT_CMD); > >> - if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) > >> + if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) || > >> + (tmp & PORT_CMD_HPCP)) > >> ap->pflags |= ATA_PFLAG_EXTERNAL; > >> } > > > > This seems to introduce a userspace regression. > > > > GNOME/udisks are now automounting internal disks, which they didn't before. > > See [0], [1], [2] > > > > ATA_PFLAG_EXTERNAL is translated into GENHD_FL_REMOVABLE. > > (Through ata_scsiop_inq_std(), scsi_add_lun(), sd_probe()) > > > > But GENHD_FL_REMOVABLE is not meant for hotpluggable devices but for > > media-changable devices (See its description in include/linux/blkdev.h). > > > > To indicate hotplug, dev_set_removable() is to be used. > > > > (Both end up in "removable" sysfs attributes, but these have different > > semantics...) > > This should take care of the issue. > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index 37ded3875ea3..170ed47ef74a 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -1912,11 +1912,8 @@ static unsigned int ata_scsiop_inq_std(struct > ata_scsi_args *args, u8 *rbuf) > 2 > }; > > - /* set scsi removable (RMB) bit per ata bit, or if the > - * AHCI port says it's external (Hotplug-capable, eSATA). > - */ > - if (ata_id_removable(args->id) || > - (args->dev->link->ap->pflags & ATA_PFLAG_EXTERNAL)) > + /* Set scsi removable (RMB) bit per ata bit. */ > + if (ata_id_removable(args->id)) > hdr[1] |= (1 << 7); > > if (args->dev->class == ATA_DEV_ZAC) { > > BUT, need to check what SAT & SATA-IO have to say about this. This is the correct fix, and we should merge it ASAP. (We set the RMB bit only if ata_id_removable() is set. ata_id_removable() is defined in CFA / CFast / Compact Flash, that can insert a card in the SATA connected reader.) The SCSI Removable Media Bit (RMB) should only be set for removable media, where the device stays and the media changes, e.g. CD-ROM or floppy. This bug was originally introduced in commit: 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") which sets the RMB bit if the port has either the eSATA bit or the hot-plug capable bit set. The reasoning was that the author wanted his eSATA ports to get treated like a USB stick. This is however wrong. See "20-082r23SPC-6: Removable Medium Bit Expectations" which has since been integrated to SPC, which states that: "Reports have been received that some USB Memory Stick device servers set the removable medium (RMB) bit to one. The rub comes when the medium is actually removed, because... The device server is removed concurrently with the medium removal. If there is no device server, then there is no device server that is waiting to have removable medium inserted. Sufficient numbers of SCSI analysts see such a device: • not as a device that supports removable medium; but • as a removable, hot pluggable device." The definition of the RMB bit in SPC was then updated to highlight this fact. Thus, a USB stick should not have the RMB bit set (and neither shall a eSATA or a hot-plug capable port). Commit dc8b4afc4a04 ("ata: ahci: don't mark HotPlugCapable Ports as external/removable") later changed so that the RMB bit is only set for the eSATA bit (and not for the hot-plug capable bit), because of the exact same problem as reported here... However, treating eSATA and hot-plug capable ports differently is of course not correct. From AHCI spec: Hot Plug Capable Port (HPCP): When set to ‘1’, indicates that this port’s signal and power connectors are externally accessible via a joint signal and power connector for blindmate device hot plug. So a hot-plug capable port is an external port, like the commit said. If we want to fix so that a eSATA port or external port is actually listed as removable, then, as Thomas said, dev_set_removable() seems to be the correct way. SCSI does have a "HOT PLUGGABLE" field in "6.7.2 Standard INQUIRY data", so the proper way to mark the SATA device as removable is probably to let ata_scsiop_inq_std() set the "HOT PLUGGABLE" field correctly (if ATA_PFLAG_EXTERNAL), such that SCSI core can call dev_set_removable() with the proper arguments. However, right now SCSI core does not call dev_set_removable() at all. In fact, it seems to only be used by drivers/mmc/core/bus.c, drivers/pci/probe.c, and drivers/usb/core/hub.c. I suggest that we: 1) Merge Damien's fix. 2) Modify SCSI to call dev_set_removable() and modify ata_scsiop_inq_std() to set the "HOT PLUGGABLE" field. Kind regards, Niklas
On Thu, Jun 13, 2024 at 03:13:54PM +0200, Niklas Cassel wrote: > On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > > On 6/13/24 15:34, Thomas Weißschuh wrote: > I suggest that we: > 1) Merge Damien's fix. This might of course result in us getting other bug reports about their distro no longer automounting their eSATA devices... and they might consider that a user space regression as well. (Since that behavior has now been there since 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable"), which was merged in 2015.) However, considering that the definition of the RMB bit has recently (2020) been clarified, it is quite clear that we are currently violating the spec, so I do still think that setting the RMB bit according to spec is the right thing to do. > 2) Modify SCSI to call dev_set_removable() and modify ata_scsiop_inq_std() > to set the "HOT PLUGGABLE" field. Kind regards, Niklas
On 2024-06-13 15:38:51+0000, Niklas Cassel wrote: > On Thu, Jun 13, 2024 at 03:13:54PM +0200, Niklas Cassel wrote: > > On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > > > On 6/13/24 15:34, Thomas Weißschuh wrote: > > > I suggest that we: > > 1) Merge Damien's fix. > > This might of course result in us getting other bug reports about their > distro no longer automounting their eSATA devices... and they might > consider that a user space regression as well. > (Since that behavior has now been there since 8a3e33cf92c7 ("ata: ahci: > find eSATA ports and flag them as removable"), which was merged in 2015.) This is quite likely. How about reverting the "ata: ahci: a hotplug capable port is an external" for now and work on a proper fix, including dev_set_removable() for an upcoming release? > However, considering that the definition of the RMB bit has recently > (2020) been clarified, it is quite clear that we are currently violating > the spec, so I do still think that setting the RMB bit according to spec > is the right thing to do. > > > > 2) Modify SCSI to call dev_set_removable() and modify ata_scsiop_inq_std() > > to set the "HOT PLUGGABLE" field. > > > Kind regards, > Niklas
On Thu, Jun 13, 2024 at 04:49:43PM +0200, Thomas Weißschuh wrote: > On 2024-06-13 15:38:51+0000, Niklas Cassel wrote: > > On Thu, Jun 13, 2024 at 03:13:54PM +0200, Niklas Cassel wrote: > > > On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > > > > On 6/13/24 15:34, Thomas Weißschuh wrote: > > > > > I suggest that we: > > > 1) Merge Damien's fix. > > > > This might of course result in us getting other bug reports about their > > distro no longer automounting their eSATA devices... and they might > > consider that a user space regression as well. > > (Since that behavior has now been there since 8a3e33cf92c7 ("ata: ahci: > > find eSATA ports and flag them as removable"), which was merged in 2015.) > > This is quite likely. > > How about reverting the "ata: ahci: a hotplug capable port is an external" > for now and work on a proper fix, including dev_set_removable() for an > upcoming release? Perhaps I'm missing something here, but how will dev_set_removable(), which sets a different sysfs attibute solve that "problem"? I think that dev_set_removable() can be added as a follow up patch, since IIUC it has nothing to do with your bug report. Calling dev_set_removable(.., DEVICE_REMOVABLE) should simply mean that the sysfs removable attribute ("fixed"/"removable"/"unknown") will be correct, so lsblk sees the device as hot-pluggable. But AFAICT, udisks will not automount the device just because the removable attribute is set to "removable". You seem to be familiar with udisks, is this understanding correct? To be honest, I think that it is wrong to automount devices just because they are hot-plug capable. eSATA and hot-plug cable ports are according to the specification both external ports, and eSATA ports are also hot-plug capable. I guess you could trigger on an uevent if a device is attached after boot, but automounting a device during boot seems wrong to me. Regardless, it seems quite clear that the RMB bit should not be set for neither eSATA nor hot-plug cable ports. Kind regards, Niklas
On 2024-06-13 17:37:51+0000, Niklas Cassel wrote: > On Thu, Jun 13, 2024 at 04:49:43PM +0200, Thomas Weißschuh wrote: > > On 2024-06-13 15:38:51+0000, Niklas Cassel wrote: > > > On Thu, Jun 13, 2024 at 03:13:54PM +0200, Niklas Cassel wrote: > > > > On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > > > > > On 6/13/24 15:34, Thomas Weißschuh wrote: > > > > > > > I suggest that we: > > > > 1) Merge Damien's fix. > > > > > > This might of course result in us getting other bug reports about their > > > distro no longer automounting their eSATA devices... and they might > > > consider that a user space regression as well. > > > (Since that behavior has now been there since 8a3e33cf92c7 ("ata: ahci: > > > find eSATA ports and flag them as removable"), which was merged in 2015.) > > > > This is quite likely. > > > > How about reverting the "ata: ahci: a hotplug capable port is an external" > > for now and work on a proper fix, including dev_set_removable() for an > > upcoming release? > > Perhaps I'm missing something here, but how will dev_set_removable(), > which sets a different sysfs attibute solve that "problem"? Indeed, it finally won't help. But only reverting that single commit should minimize the impact on users and give time to work on and discuss something better. The real solution then has time to go through a proper release cycle. > I think that dev_set_removable() can be added as a follow up patch, since > IIUC it has nothing to do with your bug report. > > Calling dev_set_removable(.., DEVICE_REMOVABLE) should simply mean that > the sysfs removable attribute ("fixed"/"removable"/"unknown") will be > correct, so lsblk sees the device as hot-pluggable. > But AFAICT, udisks will not automount the device just because the removable > attribute is set to "removable". You seem to be familiar with udisks, > is this understanding correct? From src/udiskslinuxblock.c: /* Provide easy access to _only_ the following devices * * - anything connected via known local buses (e.g. USB or Firewire, MMC or MemoryStick) * - any device with removable media * * Be careful when extending this list as we don't want to automount * the world when (inadvertently) connecting to a SAN. */ (Local policy can override this either through "noauto" in fstab or the UDISKS_AUTO udev property, but I'm not very familiar with udisks) > To be honest, I think that it is wrong to automount devices just because they > are hot-plug capable. eSATA and hot-plug cable ports are according to the > specification both external ports, and eSATA ports are also hot-plug capable. Agreed, as does udisk. > I guess you could trigger on an uevent if a device is attached after boot, > but automounting a device during boot seems wrong to me. > > Regardless, it seems quite clear that the RMB bit should not be set for > neither eSATA nor hot-plug cable ports. Ack. Thanks, Thomas
On Thu, Jun 13, 2024 at 07:33:09PM +0200, Thomas Weißschuh wrote: > On 2024-06-13 17:37:51+0000, Niklas Cassel wrote: > > On Thu, Jun 13, 2024 at 04:49:43PM +0200, Thomas Weißschuh wrote: > > > On 2024-06-13 15:38:51+0000, Niklas Cassel wrote: > > > > On Thu, Jun 13, 2024 at 03:13:54PM +0200, Niklas Cassel wrote: > > > > > On Thu, Jun 13, 2024 at 05:29:31PM +0900, Damien Le Moal wrote: > > > > > > On 6/13/24 15:34, Thomas Weißschuh wrote: > > > > > > > > > I suggest that we: > > > > > 1) Merge Damien's fix. > > > > > > > > This might of course result in us getting other bug reports about their > > > > distro no longer automounting their eSATA devices... and they might > > > > consider that a user space regression as well. > > > > (Since that behavior has now been there since 8a3e33cf92c7 ("ata: ahci: > > > > find eSATA ports and flag them as removable"), which was merged in 2015.) > > > > > > This is quite likely. > > > > > > How about reverting the "ata: ahci: a hotplug capable port is an external" > > > for now and work on a proper fix, including dev_set_removable() for an > > > upcoming release? > > > > Perhaps I'm missing something here, but how will dev_set_removable(), > > which sets a different sysfs attibute solve that "problem"? > > Indeed, it finally won't help. > But only reverting that single commit should minimize the impact on > users and give time to work on and discuss something better. Reverting is not a good solution, because that means that we will not disable LPM on hot-plug capable devices, which means that we will break hot-plug. So that would be an even more serious bug :) In my opinion, it seems quite clear that the current code is wrong (at least according to the SPC-6 specification), so I see no reason why we shouldn't just make the code spec compliant. (and if a device is not spec complinant, it should be quirked.) Damien, if you feel otherwise, please say so. Kind regards, Niklas
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index aa58ce615e79..4d3ec6d15ad1 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1648,9 +1648,10 @@ static void ahci_mark_external_port(struct ata_port *ap) void __iomem *port_mmio = ahci_port_base(ap); u32 tmp; - /* mark esata ports */ + /* mark external ports (hotplug-capable, eSATA) */ tmp = readl(port_mmio + PORT_CMD); - if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) + if (((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)) || + (tmp & PORT_CMD_HPCP)) ap->pflags |= ATA_PFLAG_EXTERNAL; }