diff mbox series

scsi: pm80xx: Fix 'Unknown' max/min linkrate

Message ID 20220707175210.528858-1-changyuanl@google.com
State New
Headers show
Series scsi: pm80xx: Fix 'Unknown' max/min linkrate | expand

Commit Message

Changyuan Lyu July 7, 2022, 5:52 p.m. UTC
Currently, the dataflow of the max/min linkrate in the driver is
* in pm8001_get_lrate_mode():
  hardcoded value ==> struct sas_phy
* in pm8001_bytes_dmaed():
  struct pm8001_phy ==> struct sas_phy
* in pm8001_phy_control():
  libsas data ==> struct pm8001_phy

Since pm8001_bytes_dmaed() follows pm8001_get_lrate_mode(), and
the fields in struct pm8001_phy are not initialized, sysfs
`/sys/class/sas_phy/phy-*/maximum_linkrate` always shows `Unknown`.

To fix the issue, this patch changes the dataflow to the following:
* in pm8001_phy_init():
  initial value ==> struct pm8001_phy
* in pm8001_get_lrate_mode():
  struct pm8001_phy ==> struct sas_phy
* in pm8001_phy_control():
  libsas data ==> struct pm8001_phy

For negotiated linkrate, the current dataflow is
* in pm8001_get_lrate_mode():
  iomb data ==> struct asd_sas_phy ==> struct sas_phy
* in pm8001_bytes_dmaed():
  struct asd_sas_phy ==> struct sas_phy

Since pm8001_bytes_dmaed() follows pm8001_get_lrate_mode(), the
assignment statements in pm8001_bytes_dmaed() are unnecessary and
cleaned up.

Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c  | 19 +++----------------
 drivers/scsi/pm8001/pm8001_init.c |  2 ++
 2 files changed, 5 insertions(+), 16 deletions(-)

Comments

Jinpu Wang July 11, 2022, 5:44 a.m. UTC | #1
On Thu, Jul 7, 2022 at 7:52 PM Changyuan Lyu <changyuanl@google.com> wrote:
>
> Currently, the dataflow of the max/min linkrate in the driver is
> * in pm8001_get_lrate_mode():
>   hardcoded value ==> struct sas_phy
> * in pm8001_bytes_dmaed():
>   struct pm8001_phy ==> struct sas_phy
> * in pm8001_phy_control():
>   libsas data ==> struct pm8001_phy
>
> Since pm8001_bytes_dmaed() follows pm8001_get_lrate_mode(), and
> the fields in struct pm8001_phy are not initialized, sysfs
> `/sys/class/sas_phy/phy-*/maximum_linkrate` always shows `Unknown`.
>
> To fix the issue, this patch changes the dataflow to the following:
> * in pm8001_phy_init():
>   initial value ==> struct pm8001_phy
> * in pm8001_get_lrate_mode():
>   struct pm8001_phy ==> struct sas_phy
> * in pm8001_phy_control():
>   libsas data ==> struct pm8001_phy
>
> For negotiated linkrate, the current dataflow is
> * in pm8001_get_lrate_mode():
>   iomb data ==> struct asd_sas_phy ==> struct sas_phy
> * in pm8001_bytes_dmaed():
>   struct asd_sas_phy ==> struct sas_phy
>
> Since pm8001_bytes_dmaed() follows pm8001_get_lrate_mode(), the
> assignment statements in pm8001_bytes_dmaed() are unnecessary and
> cleaned up.
>
> Signed-off-by: Changyuan Lyu <changyuanl@google.com>
> Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
looks it was broken since beginning, would be good to add cc stable.
Thx!
Acked-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c  | 19 +++----------------
>  drivers/scsi/pm8001/pm8001_init.c |  2 ++
>  2 files changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index f7466a895d3b..991eb01bb1e0 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -3145,15 +3145,6 @@ void pm8001_bytes_dmaed(struct pm8001_hba_info *pm8001_ha, int i)
>         if (!phy->phy_attached)
>                 return;
>
> -       if (sas_phy->phy) {
> -               struct sas_phy *sphy = sas_phy->phy;
> -               sphy->negotiated_linkrate = sas_phy->linkrate;
> -               sphy->minimum_linkrate = phy->minimum_linkrate;
> -               sphy->minimum_linkrate_hw = SAS_LINK_RATE_1_5_GBPS;
> -               sphy->maximum_linkrate = phy->maximum_linkrate;
> -               sphy->maximum_linkrate_hw = phy->maximum_linkrate;
> -       }
> -
>         if (phy->phy_type & PORT_TYPE_SAS) {
>                 struct sas_identify_frame *id;
>                 id = (struct sas_identify_frame *)phy->frame_rcvd;
> @@ -3177,26 +3168,22 @@ void pm8001_get_lrate_mode(struct pm8001_phy *phy, u8 link_rate)
>         switch (link_rate) {
>         case PHY_SPEED_120:
>                 phy->sas_phy.linkrate = SAS_LINK_RATE_12_0_GBPS;
> -               phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_12_0_GBPS;
>                 break;
>         case PHY_SPEED_60:
>                 phy->sas_phy.linkrate = SAS_LINK_RATE_6_0_GBPS;
> -               phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_6_0_GBPS;
>                 break;
>         case PHY_SPEED_30:
>                 phy->sas_phy.linkrate = SAS_LINK_RATE_3_0_GBPS;
> -               phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_3_0_GBPS;
>                 break;
>         case PHY_SPEED_15:
>                 phy->sas_phy.linkrate = SAS_LINK_RATE_1_5_GBPS;
> -               phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_1_5_GBPS;
>                 break;
>         }
>         sas_phy->negotiated_linkrate = phy->sas_phy.linkrate;
> -       sas_phy->maximum_linkrate_hw = SAS_LINK_RATE_6_0_GBPS;
> +       sas_phy->maximum_linkrate_hw = phy->maximum_linkrate;
>         sas_phy->minimum_linkrate_hw = SAS_LINK_RATE_1_5_GBPS;
> -       sas_phy->maximum_linkrate = SAS_LINK_RATE_6_0_GBPS;
> -       sas_phy->minimum_linkrate = SAS_LINK_RATE_1_5_GBPS;
> +       sas_phy->maximum_linkrate = phy->maximum_linkrate;
> +       sas_phy->minimum_linkrate = phy->minimum_linkrate;
>  }
>
>  /**
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 9b04f1a6a67d..01f2f41928eb 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -143,6 +143,8 @@ static void pm8001_phy_init(struct pm8001_hba_info *pm8001_ha, int phy_id)
>         struct asd_sas_phy *sas_phy = &phy->sas_phy;
>         phy->phy_state = PHY_LINK_DISABLE;
>         phy->pm8001_ha = pm8001_ha;
> +       phy->minimum_linkrate = SAS_LINK_RATE_1_5_GBPS;
> +       phy->maximum_linkrate = SAS_LINK_RATE_6_0_GBPS;
>         sas_phy->enabled = (phy_id < pm8001_ha->chip->n_phy) ? 1 : 0;
>         sas_phy->class = SAS;
>         sas_phy->iproto = SAS_PROTOCOL_ALL;
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
Martin K. Petersen July 14, 2022, 4:22 a.m. UTC | #2
On Thu, 7 Jul 2022 10:52:10 -0700, Changyuan Lyu wrote:

> Currently, the dataflow of the max/min linkrate in the driver is
> * in pm8001_get_lrate_mode():
>   hardcoded value ==> struct sas_phy
> * in pm8001_bytes_dmaed():
>   struct pm8001_phy ==> struct sas_phy
> * in pm8001_phy_control():
>   libsas data ==> struct pm8001_phy
> 
> [...]

Applied to 5.19/scsi-fixes, thanks!

[1/1] scsi: pm80xx: Fix 'Unknown' max/min linkrate
      https://git.kernel.org/mkp/scsi/c/e78276cadb66
diff mbox series

Patch

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index f7466a895d3b..991eb01bb1e0 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -3145,15 +3145,6 @@  void pm8001_bytes_dmaed(struct pm8001_hba_info *pm8001_ha, int i)
 	if (!phy->phy_attached)
 		return;
 
-	if (sas_phy->phy) {
-		struct sas_phy *sphy = sas_phy->phy;
-		sphy->negotiated_linkrate = sas_phy->linkrate;
-		sphy->minimum_linkrate = phy->minimum_linkrate;
-		sphy->minimum_linkrate_hw = SAS_LINK_RATE_1_5_GBPS;
-		sphy->maximum_linkrate = phy->maximum_linkrate;
-		sphy->maximum_linkrate_hw = phy->maximum_linkrate;
-	}
-
 	if (phy->phy_type & PORT_TYPE_SAS) {
 		struct sas_identify_frame *id;
 		id = (struct sas_identify_frame *)phy->frame_rcvd;
@@ -3177,26 +3168,22 @@  void pm8001_get_lrate_mode(struct pm8001_phy *phy, u8 link_rate)
 	switch (link_rate) {
 	case PHY_SPEED_120:
 		phy->sas_phy.linkrate = SAS_LINK_RATE_12_0_GBPS;
-		phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_12_0_GBPS;
 		break;
 	case PHY_SPEED_60:
 		phy->sas_phy.linkrate = SAS_LINK_RATE_6_0_GBPS;
-		phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_6_0_GBPS;
 		break;
 	case PHY_SPEED_30:
 		phy->sas_phy.linkrate = SAS_LINK_RATE_3_0_GBPS;
-		phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_3_0_GBPS;
 		break;
 	case PHY_SPEED_15:
 		phy->sas_phy.linkrate = SAS_LINK_RATE_1_5_GBPS;
-		phy->sas_phy.phy->negotiated_linkrate = SAS_LINK_RATE_1_5_GBPS;
 		break;
 	}
 	sas_phy->negotiated_linkrate = phy->sas_phy.linkrate;
-	sas_phy->maximum_linkrate_hw = SAS_LINK_RATE_6_0_GBPS;
+	sas_phy->maximum_linkrate_hw = phy->maximum_linkrate;
 	sas_phy->minimum_linkrate_hw = SAS_LINK_RATE_1_5_GBPS;
-	sas_phy->maximum_linkrate = SAS_LINK_RATE_6_0_GBPS;
-	sas_phy->minimum_linkrate = SAS_LINK_RATE_1_5_GBPS;
+	sas_phy->maximum_linkrate = phy->maximum_linkrate;
+	sas_phy->minimum_linkrate = phy->minimum_linkrate;
 }
 
 /**
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 9b04f1a6a67d..01f2f41928eb 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -143,6 +143,8 @@  static void pm8001_phy_init(struct pm8001_hba_info *pm8001_ha, int phy_id)
 	struct asd_sas_phy *sas_phy = &phy->sas_phy;
 	phy->phy_state = PHY_LINK_DISABLE;
 	phy->pm8001_ha = pm8001_ha;
+	phy->minimum_linkrate = SAS_LINK_RATE_1_5_GBPS;
+	phy->maximum_linkrate = SAS_LINK_RATE_6_0_GBPS;
 	sas_phy->enabled = (phy_id < pm8001_ha->chip->n_phy) ? 1 : 0;
 	sas_phy->class = SAS;
 	sas_phy->iproto = SAS_PROTOCOL_ALL;