[PATCH] scsi: mpi3mr: use DevicePage0 link rate only for direct-attached targets
Ilya Khomyakov <[email protected]> Tue, 4 Aug 2026 17:28:00 +0300
| Newsgroups | gmane.linux.scsi,gmane.linux.kernel |
|---|---|
| Message-ID | <[email protected]> |
This patch fixes a regression in the Broadcom MPI3 Storage Controller
driver under drivers/scsi/mpi3mr/.
Commit c273c14b0294 ("scsi: mpi3mr: Use negotiated link rate from
DevicePage0") changed mpi3mr to prefer the cached DevicePage0 rate.
DevicePage0 stores negotiated_link_rate as one standalone MPI3 SAS
link-rate code, while SAS PHY Page 0 and SAS Expander Page 1 store logical
and physical rates as two nibbles of one packed byte.
mpi3mr_get_sas_negotiated_logical_linkrate() currently sends both formats
through the same high-nibble extraction at its common exit. A valid
DevicePage0 value of 0x0b or 0x0c is therefore converted to zero. The later
minimum-rate safeguard then publishes the value as 1.5 Gbit/s.
For example:
DevicePage0 value: 0x0b
packed-field extraction: (0x0b & 0xf0) >> 4 = 0x00
minimum-rate substitution: 0x00 -> 0x08
Linux SAS transport result: 1.5 Gbit/s
There is also a topology distinction. The caller uses the returned value to
update the parent sas_phy. In an expander topology with the HBA-to-expander
link limited to 12 Gbit/s, DevicePage0 reported 0x0b for every tested
target, while Expander Page 1 reported 0xcc for several disk-facing PHYs
that were operating at 22.5 Gbit/s. The cached target value therefore
cannot unconditionally replace the local parent expander PHY value.
The original failure was reproduced on an eHBA 9600 controller with a
SAS4016 IOC and a 46-PHY Microchip expander. Before the fix, a target with
DevicePage0 negotiated_link_rate 0x0b could remain visible in sysfs as
1.5 Gbit/s when a replayed topology event contained current=0x0b and
previous=0x0b and therefore skipped the Linux-side update.
Validate the standalone DevicePage0 code and use it directly only for a
directly attached target. For an expander-attached target, retain the
existing Expander Page 1 path because it describes the local parent
expander PHY being updated. Retain the SAS PHY Page 0 fallback when a
directly attached cached value is invalid.
The topology-aware variant was tested in an out-of-tree mpi3mr 8.17.1.0.
build. After boot, Linux reported:
PHY 18: 12.0 Gbit/s
PHY 19: 12.0 Gbit/s
PHY 24: 22.5 Gbit/s
PHY 25: 22.5 Gbit/s
PHY 26: 22.5 Gbit/s
PHY 27: 22.5 Gbit/s
PHY 28: 12.0 Gbit/s
PHY 30: 22.5 Gbit/s
No tested PHY was incorrectly reported as 1.5 Gbit/s, and the 22.5 Gbit/s
disk-facing rates were preserved even though DevicePage0 contained 0x0b.
Fixes: c273c14b0294 ("scsi: mpi3mr: Use negotiated link rate from DevicePage0")
Signed-off-by: Ilya Khomyakov <[email protected]>
---
drivers/scsi/mpi3mr/mpi3mr_transport.c | 46 ++++++++++++++++++++++----
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_transport.c b/drivers/scsi/mpi3mr/mpi3mr_transport.c
index 240f67a..740fccc 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_transport.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_transport.c
@@ -587,6 +587,30 @@ static enum sas_linkrate mpi3mr_convert_phy_link_rate(u8 link_rate)
return rc;
}
+/**
+ * mpi3mr_sas_link_rate_valid - validate a standalone SAS link-rate code
+ * @link_rate: MPI3 standalone SAS negotiated link-rate code
+ *
+ * DevicePage0 stores one plain negotiated link-rate code. Accept only
+ * negotiated data rates supported by the SAS transport conversion code;
+ * reserved or transitional values must use the existing PHY-page path.
+ *
+ * Return: true for a supported negotiated data rate, false otherwise.
+ */
+static bool mpi3mr_sas_link_rate_valid(u8 link_rate)
+{
+ switch (link_rate) {
+ case MPI3_SAS_NEG_LINK_RATE_1_5:
+ case MPI3_SAS_NEG_LINK_RATE_3_0:
+ case MPI3_SAS_NEG_LINK_RATE_6_0:
+ case MPI3_SAS_NEG_LINK_RATE_12_0:
+ case MPI3_SAS_NEG_LINK_RATE_22_5:
+ return true;
+ default:
+ return false;
+ }
+}
+
/**
* mpi3mr_delete_sas_phy - Remove a single phy from port
* @mrioc: Adapter instance reference
@@ -2292,18 +2316,26 @@ void mpi3mr_expander_remove(struct mpi3mr_ioc *mrioc, u64 sas_address,
static u8 mpi3mr_get_sas_negotiated_logical_linkrate(struct mpi3mr_ioc *mrioc,
struct mpi3mr_tgt_dev *tgtdev)
{
- u8 link_rate = MPI3_SAS_NEG_LINK_RATE_1_5, phy_number;
+ u8 cached_link_rate, link_rate = MPI3_SAS_NEG_LINK_RATE_1_5;
+ u8 phy_number;
struct mpi3_sas_expander_page1 expander_pg1;
struct mpi3_sas_phy_page0 phy_pg0;
u32 phynum_handle;
u16 ioc_status;
- /* First, try to use link rate from DevicePage0 (populated by firmware) */
- if (tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate >=
- MPI3_SAS_NEG_LINK_RATE_1_5) {
- link_rate = tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate;
- goto out;
- }
+ cached_link_rate =
+ tgtdev->dev_spec.sas_sata_inf.negotiated_link_rate;
+
+ /*
+ * For a directly attached target, DevicePage0 and the parent host PHY
+ * describe the same link, so the standalone cached code can be used
+ * without packed-field decoding. For an expander-attached target, the
+ * caller updates the parent expander PHY and DevicePage0 can differ from
+ * that local segment; retain the Expander Page 1 read in that case.
+ */
+ if ((tgtdev->devpg0_flag & MPI3_DEVICE0_FLAGS_ATT_METHOD_DIR_ATTACHED) &&
+ mpi3mr_sas_link_rate_valid(cached_link_rate))
+ return cached_link_rate;
/* Fallback to reading from phy pages if DevicePage0 value not available */
phy_number = tgtdev->dev_spec.sas_sata_inf.phy_id;