Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <549277c9-d3c8-41ae-afe1-c6860295ec26@oracle.com>
Date:   Mon, 4 Dec 2023 13:17:51 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 2/3] scsi: libsas: Move sas_add_parent_port() to
 sas_expander.c
Content-Language: en-US
To:     Xingui Yang <yangxingui@huawei.com>, yanaijie@huawei.com,
        jejb@linux.ibm.com, martin.petersen@oracle.com,
        damien.lemoal@opensource.wdc.com
Cc:     linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linuxarm@huawei.com, prime.zeng@hisilicon.com,
        chenxiang66@hisilicon.com, kangfenglong@huawei.com
References: <20231204122932.55741-1-yangxingui@huawei.com>
 <20231204122932.55741-3-yangxingui@huawei.com>
From:   John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <20231204122932.55741-3-yangxingui@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Tk5YTTRtMG5ZRWVEYVgyU2hPUkJrZEVzN1o3ek5yRVRVWmNzTGRQRDNYZStw?=
 =?utf-8?B?RGJneHhGb2s2SENySnB5dWJVS2NXTFJSMGd1K1RVc3ZLbzUxUGVReGkweTVL?=
 =?utf-8?B?MW16OEdhUWlROWVwUmdFMk5uYlhIUXovdFRVU3lwaXV1Y0pucHY3cC9Vbkpm?=
 =?utf-8?B?Z3I2bkJheVdWYy84M2lXUzlVc05ObmdSUUhhVHlRdFBiZzZDS1cyRkRCczc3?=
 =?utf-8?B?b0k5OU1YZTJ1MFN6azZlMkkrSkNobVBwVEt3MXRFM2pPaWNodExUbGxGbVJO?=
 =?utf-8?B?YWVlR0xERWlCM3BQWEliem9hbS9SSVQvZHUwMDBuQmo4ZGZJNmN3YWhnOWxV?=
 =?utf-8?B?YXFobitPM2puQzVOVjFsc29MT09yN3JxelFZRXFYVnhTOVRDdHBvc1F2cGpN?=
 =?utf-8?B?cjdkSkZuNElHbnl2TXRISmV0SVUzVlpHTEZ1ZE5Qb1RvRVpreDlvTGoxRStU?=
 =?utf-8?B?NlpXbFlLZVBXN21WTnBRSnFmdnVIVXRVUnR0d2Q5L3Y1VUpWWUI4R2ovZmlw?=
 =?utf-8?B?VlBmSVJXekFFR1JIanZTeStaSXl3c2czTWZTeUFLMmUrVStmYUFzcllmQ2Iw?=
 =?utf-8?B?NHN3QkthWFQzSS9GYjlYdmtsL0Y2TVYrNU5hblhLUDFGby9qUlNOUUpmSDV1?=
 =?utf-8?B?ckF3eXE3bER1NkNmUnBTRWdqSC9KZm9HL0ZoNnVtSndoTFhMRHRXU2ZIaDdX?=
 =?utf-8?B?V2RNaG5NdVVTK3B0dHFINzR1blVhbFhNTEJmMFJrbmw1d2FSbDRxUGdlUWpC?=
 =?utf-8?B?MG9uYU9JaElMSnRSWXc1RWJMT0F3Ym5FMFE5STdHS3BPdWZZRXl4WDh6M05O?=
 =?utf-8?B?V0FVM0N1UXNiZXhNdUJ6K3NzSFlWd2JzMng0OTdrUFFsdDk0SjhuVUdOeHJm?=
 =?utf-8?B?dXpGL001a2ZVMEZYc2VIT2JJMmY5blAzRzZhWVllTFhEK0kwZkZYL09vbWtT?=
 =?utf-8?B?OXRrTVMwR2xZbnJkK3NCUzkySnNtcW91eUFKYlN6aHFpL3poNTBHellnVHA3?=
 =?utf-8?B?UGhaUzBES0tJVk5GdFhELzh5OGVNZ1plVkY3TGNSdjdUa2QzdEZUK0VSQlYw?=
 =?utf-8?B?WURabmRCUWU5dmI1ZUV6SEZoZTVzZEZOZHV3MVkwczlqSGdGZ3c5cUdLeVZG?=
 =?utf-8?B?THVYN29leWdPWFllVWY1eTNjbzBoanIvWDk5SUFHbG5FVW0xS3k5NnQ4d0dK?=
 =?utf-8?B?RUFKelExUkRhUDZWTHllQ21Ib1FhQWlwNjk1VW5tbXlwdm4rOGdKWTJpbjlG?=
 =?utf-8?B?UUNwWXVBTG9CM2gyRnNEWmdRcWRDVURtcmRKUXRVeVptRkR1blA4UjN3UG5w?=
 =?utf-8?B?cnpWTVp4bzNlQ1Z3ZnhJbDRNWDl4ajFhWDlFOVVZYUgwbGwwaGJVRU5BVUlq?=
 =?utf-8?B?SG1MT1QzOHhHVElkSlcybklYT2FKamgrQlM3L2w4MnVhL0FoT2hCZzIyaFdn?=
 =?utf-8?B?Q2c5NGdvam1Wd25jMitjNWxmVmZqSnJPNmVCcTl2dHBqUnJIWUVtcnJ6Qm5m?=
 =?utf-8?B?MFdLUGZCSWN4b0R0bGZGb0lFZXpldWVnT1ZmRzJ2UHh6Y2E4WTl3UUptZHVv?=
 =?utf-8?B?d2JrdGlpWkNoNVBXalBwUmZBcXZGRVhJWHFESGhQWG9ZaVBBZURsYWt3M0Nw?=
 =?utf-8?B?OG51cmo2c2M4QVdkU29FczgrZXByVzZlMEw1VTI4b0JhN28zRWhjOFNScUN2?=
 =?utf-8?B?SWdsQkdTWDRCMDZ1ZUE0N2luVEdoVzk5UDBnMVU3enFrUzNhNVh1Z1c4bU4x?=
 =?utf-8?B?Ri9hNGFlQjRyNTlqTWR6RW9FdG1zM0Y5ZDhvVzNSUXhjSTZnbVV2TlNNQ212?=
 =?utf-8?B?YVlxMmRPdWFvTFBqVUNhZmtLWFRzSThaZDZzY1NDQm1KYytxVGwxay90dEZT?=
 =?utf-8?B?dE1UM21HZSsxV1hsMC9SbjN3VlpOYk5sYThxbklKc0FMMzk5S291OFNYaWVD?=
 =?utf-8?B?WEZ0ZTMzYTZxWlNDU3I3eGdzOTNPTnFtdGU0QVpPbDE0SGg5MlpXYTRzYTdr?=
 =?utf-8?B?WmsvdFY3L1lpa3Y1V3pBOTMxdmUwcUk3RHBZS0luQndKTlNkQVZZRUkrN0dI?=
 =?utf-8?B?VzI1T3owRUJQcTZlRWRYYi9ZQnJ1OW9vcHlEVWFqZEkzWHlhS0s3ZC91NkZo?=
 =?utf-8?Q?CdoOAChp6ADLUxHC3X9PUFgc3?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: LhsTiih+Dyhs9lKS8Ia1MSov0ndaDBv0HfLpBcGeHQg8KfcDP4h0LtPta7qwv7oA9+G5q7teF+79RyTRseyQ/o7TAms1s9+A1qAYLA8Y+xpK7LWsyeypYYg7bwu4HqcfrHkwS0Y4o+sWvBLjTydALbep2tsBUrGvj8EaPxLuiBmFdtKzVPkct/3Lt/KaA8tlg+jlCznkCxwGALCNP069p6Ckb5nxNzy3nm7fOfwlN/9sF69T+e9lCpdB7mETPlyiMXsZyPXv1q/fumJLdkZWcKACO1t7dbnAV/qsghDbNCIG2n8/5LGuif8ibqdZT4drv4tSRr/VIxHoCZ4nPG8t2PqWWdUdKOuRiw6Dsz8ZLE2n4Vmu2pYmLRxaVEycix/v1SC8+qXFWnyaeaaaXm2KmSKqSaySZqa75i7NWNcC7p3sER78cZ+lVB8G4uIoNyqnxqPK3pt7EIg/MVQOey7X2lBlFJ7tcFzL4u+m/x0OmAoOJP/XnkSr0N+rnL400NBoyLJN9l/qVEu5SXXv8Oxs2wiNGx0+IKzc+9gBIaJBA1OLcbFlGOvkqWQpUvfKKNNMGsvRceYzsY0UtNwBm3kADmiINAepNyowpXEBUFQ+TqpROv0MAk+sOMEcNTyIQ/1rZHslJ21U2vOWajW/YvUNuINLdKlxyMYLm663X+y4iZk2Lxv/gSiZNH5EhIMXs5tTxAoTukSYZt/oOcb95TR833Ik6+BPBxSfv6bL84NM7nX6T5hHslpNL/Mla1fR4SNosix11Py9jVAk+dmIx2IbcIX3opdqvFzJMLD+XgKwzlK77NC+/iB9Bvv8uuo5z6WwfmxQ28HSxXtoCcskntjYk7whBHckIDwT0qAHqBpo2adx2PGlB7ddlmWxuGuQgLBav0wYss1cAKumP1MREsWJ392JOQLRthYvyi/ri6cUhg8=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3352029f-a536-47ff-3ace-08dbf4cb6ceb
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Dec 2023 13:17:54.7848
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: pgKhsx4aPgOG5hPuJGynPSiqsvuLe3ojE+s3WBPSKoh4LHh9ic4PT9MO6j9YO0VRHd2z2nTnJVjm6FKddJb15w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB5750
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-04_11,2023-12-04_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 malwarescore=0
 mlxlogscore=999 mlxscore=0 suspectscore=0 adultscore=0 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311060000 definitions=main-2312040099
X-Proofpoint-GUID: rB2_YzImKDoXQekXT4AN7_tPcJMaXSCE
X-Proofpoint-ORIG-GUID: rB2_YzImKDoXQekXT4AN7_tPcJMaXSCE
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Mon, 04 Dec 2023 05:18:30 -0800 (PST)

On 04/12/2023 12:29, Xingui Yang wrote:
> Move sas_add_parent_port() to sas_expander.c since it is only used in this
> file.
> 
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>

Ignoring some comments, below:
Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   drivers/scsi/libsas/sas_expander.c | 15 +++++++++++++++
>   drivers/scsi/libsas/sas_internal.h | 15 ---------------
>   2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 1257f90130fb..7aa968b85e1e 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -33,6 +33,21 @@ static void sas_port_add_ex_phy(struct sas_port *port, struct ex_phy *ex_phy)
>   	ex_phy->phy_state = PHY_DEVICE_DISCOVERED;
>   }
>   
> +static void sas_add_parent_port(struct domain_device *dev, int phy_id)

Can we have "_ex" in the name, like sas_ex_add_parent_port()

> +{
> +	struct expander_device *ex = &dev->ex_dev;
> +	struct ex_phy *ex_phy = &ex->ex_phy[phy_id];
> +
> +	if (!ex->parent_port) {
> +		ex->parent_port = sas_port_alloc(&dev->rphy->dev, phy_id);
> +		/* FIXME: error handling */
> +		BUG_ON(!ex->parent_port);
> +		BUG_ON(sas_port_add(ex->parent_port));

how about fixing these at some stage?

> +		sas_port_mark_backlink(ex->parent_port);
> +	}
> +	sas_port_add_phy(ex->parent_port, ex_phy->phy);
> +}
> +
>   /* ---------- SMP task management ---------- */
>   
>   /* Give it some long enough timeout. In seconds. */
> diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
> index 3804aef165ad..85948963fb97 100644
> --- a/drivers/scsi/libsas/sas_internal.h
> +++ b/drivers/scsi/libsas/sas_internal.h
> @@ -189,21 +189,6 @@ static inline void sas_phy_set_target(struct asd_sas_phy *p, struct domain_devic
>   	}
>   }
>   
> -static inline void sas_add_parent_port(struct domain_device *dev, int phy_id)
> -{
> -	struct expander_device *ex = &dev->ex_dev;
> -	struct ex_phy *ex_phy = &ex->ex_phy[phy_id];
> -
> -	if (!ex->parent_port) {
> -		ex->parent_port = sas_port_alloc(&dev->rphy->dev, phy_id);
> -		/* FIXME: error handling */
> -		BUG_ON(!ex->parent_port);
> -		BUG_ON(sas_port_add(ex->parent_port));
> -		sas_port_mark_backlink(ex->parent_port);
> -	}
> -	sas_port_add_phy(ex->parent_port, ex_phy->phy);
> -}
> -
>   static inline struct domain_device *sas_alloc_device(void)
>   {
>   	struct domain_device *dev = kzalloc(sizeof(*dev), GFP_KERNEL);