Re: [PATCH v2 05/15] scsi: hisi_sas: Fix some issues related to asd_sas_port->phy_list

From: "chenxiang (M)" <chenxiang66@hisilicon.com>
To: John Garry <john.garry@huawei.com>,
	Nathan Chancellor <nathan@kernel.org>,
	"martin.petersen@oracle.com" <martin.petersen@oracle.com>
Cc: "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Linuxarm <linuxarm@huawei.com>,
	"llvm@lists.linux.dev" <llvm@lists.linux.dev>,
	"colin.i.king@gmail.com" <colin.i.king@gmail.com>
Subject: Re: [PATCH v2 05/15] scsi: hisi_sas: Fix some issues related to asd_sas_port->phy_list
Date: Tue, 4 Jan 2022 19:38:48 +0800	[thread overview]
Message-ID: <ad62d555-971b-8868-3c9f-be68b485a17d@hisilicon.com> (raw)
In-Reply-To: <1aae302f-08d9-87e8-6e0f-4b146de8d9c9@huawei.com>

Hi John,

在 2022/1/4 18:02, John Garry 写道:
>>
>> @Martin and @John Garry, could you have a review and consider to merge
>> following patch ?
>
> This series has not made it to Martin's 5.17 queue, but I suggest that 
> you send it as a patch in case it does.

Ok, i will send it as a patch.

>
>>
>> From: Xiang Chen <chenxiang66@hisilicon.com>
>> Date: Tue, 28 Dec 2021 09:40:01 +0800
>> Subject: [PATCH] scsi: libsas: Remove unused variable and check in 
>> function
>>    hisi_sas_send_ata_reset_each_phy()
>
> please try to condense these subjects, like just "Remove broken legacy 
> code in hisi_sas_send_ata_reset_each_phy()"
>
> Or at least remove "function", as this is obvious

I will remove "function".

>
>>
>> In commit 29e2bac87421 ("scsi: hisi_sas: Fix some issues related to
>> asd_sas_port->phy_list"), we use asd_sas_port->phy_mask instead of
>> accessing asd_sas_port->phy_list, and it is enough to use
>> asd_sas_port->phy_mask to check the state of phy, so removing the
>
> /s/removing/remove/

Ok

>
>> old and unused check.
>>
>> Fixes: 29e2bac87421 ("scsi: hisi_sas: Fix some issues related to
>> asd_sas_port->phy_list")
>> Reported-by: Nathan Chancellor <nathan@kernel.org>
>
> Colin King also reported this, so please add him.

Right, i will add him.

>
>> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
>> ---
>>    drivers/scsi/hisi_sas/hisi_sas_main.c | 5 -----
>>    1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> index f46f679fe825..a05ec7aece5a 100644
>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
>> @@ -1525,16 +1525,11 @@ static void
>> hisi_sas_send_ata_reset_each_phy(struct hisi_hba *hisi_hba,
>>           struct device *dev = hisi_hba->dev;
>>           int s = sizeof(struct host_to_dev_fis);
>>           int rc = TMF_RESP_FUNC_FAILED;
>> -       struct asd_sas_phy *sas_phy;
>>           struct ata_link *link;
>>           u8 fis[20] = {0};
>> -       u32 state;
>>           int i;
>>
>> -       state = hisi_hba->hw->get_phys_state(hisi_hba);
>>           for (i = 0; i < hisi_hba->n_phy; i++) {
>> -               if (!(state & BIT(sas_phy->id)))
>> -                       continue;
>>                   if (!(sas_port->phy_mask & BIT(i)))
>>                           continue;
>>
>> -- 
>> 2.33.0
>>
>>
>>
>
> .
>