Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Bean Huo <beanhuo@iokpp.de>,
        "alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
        "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "stanley.chu@mediatek.com" <stanley.chu@mediatek.com>,
        "beanhuo@micron.com" <beanhuo@micron.com>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "tomas.winkler@intel.com" <tomas.winkler@intel.com>,
        "daejun7.park@samsung.com" <daejun7.park@samsung.com>,
        "quic_cang@quicinc.com" <quic_cang@quicinc.com>,
        "quic_nguyenb@quicinc.com" <quic_nguyenb@quicinc.com>,
        "quic_xiaosenh@quicinc.com" <quic_xiaosenh@quicinc.com>,
        "quic_richardp@quicinc.com" <quic_richardp@quicinc.com>,
        "quic_asutoshd@quicinc.com" <quic_asutoshd@quicinc.com>,
        "hare@suse.de" <hare@suse.de>
CC:     "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v2 3/6] ufs: core: Split ufshcd_map_sg
Thread-Topic: [PATCH v2 3/6] ufs: core: Split ufshcd_map_sg
Thread-Index: AQHY/S6bmPmPCTCtbEC2ZN34Ub/ena5KlgAw
Date:   Tue, 22 Nov 2022 08:15:36 +0000
Message-ID: <DM6PR04MB65756D4E5DEAAA95FCA24784FC0D9@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20221120222217.108492-1-beanhuo@iokpp.de>
 <20221120222217.108492-4-beanhuo@iokpp.de>
In-Reply-To: <20221120222217.108492-4-beanhuo@iokpp.de>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?pQIBWTG+EMx8fU7W6gqCoP3zPj0Ig9X2SlMYs8GPk8s8n5CkUyC16w9kB/IP?=
 =?us-ascii?Q?NxiXipo2OjZaADg4In4DbNUne4q2dagldTxHwnWr7Qi1t+gbZYZnqvfYSz6z?=
 =?us-ascii?Q?PqDpYTgHwL1Rx+suIMByfzO/sP9+YMb3SyYPY9I0c8UTwcQ30ApVTwStcxEX?=
 =?us-ascii?Q?sdPYcjwv6ZMp43/+qoonJAfd1XjpO/l/VUj3z07uDuqV0Vjwl43kyKqd9rTP?=
 =?us-ascii?Q?Z0d1BBIi+UhO4xMQLk0fYXgMdvrBlC8F4sYCf/Mv9SKkLaBZWXImmBzdN6X1?=
 =?us-ascii?Q?+EgJQ2Nc3drnOBeJjq4mH36BuZx4+7ayarCN3Z87O6QjpjmJjc7tzky6N53G?=
 =?us-ascii?Q?+eFCoBo62UyoqfYa4H42xvlKHTOGWvZqbfvw2CGp4JHVXkIuGSvuKSUZ7Zzx?=
 =?us-ascii?Q?cljchh5xPXO3/MFtfEQi0mjzgCT4meuxL2/h++sNLsyDtpBmSNtA++deCuUA?=
 =?us-ascii?Q?yQ5417/hqmAvGnxEwG46jHt8WJoyW2PQAwVsbhpbABRYYOg54h5hyhiI3B/X?=
 =?us-ascii?Q?TKKY80FsKkwM7y6WuxXwKiHx45tBOnsMEMe3c5ufAU+cK5ILR7amFIuO4/B5?=
 =?us-ascii?Q?w0zhGwzTHlDphB5nJ3VzLESmENtrcZ5Bp+cRqAGcFrggZU7GXTVQ8rdgm6FU?=
 =?us-ascii?Q?kcs/ETLnrmJoDS0gFSJ0JveLUx6OpT43bMHC2X5v28NGLh0LRwNT/MrB9Ydv?=
 =?us-ascii?Q?rpCkPansqVULrmHHr9dhuiK1/KGEhg3mhFpGCaj7uFUnCr4df/70193NxXQl?=
 =?us-ascii?Q?OR1nxCOf4zlwuD+VypS4pRprvEvd06hujPR0818nhb3LccWMVaIUyqnQEkT9?=
 =?us-ascii?Q?Ty6GpgbU7F6+83exHwKTfX8/GZpoT2uKrSTeqQJAIVDDi5q3Jkq7xV6VDQ/2?=
 =?us-ascii?Q?5bboZ+l5jW76D9v9mMCUdk2vkhp6RNvUNR7GH/P6BLBHGXWneahp3ecxcmiT?=
 =?us-ascii?Q?VXe5V3CHo1cvGdSbVmJs6tbbAzswVkcXSHTlfD99AhA1pEjztzTf7V/621LV?=
 =?us-ascii?Q?OWRyAgZK5Ol77Elikfd3ydVQJkalL+Ze7CPX/nH0gKPhHKJQzvNz110q0ZI5?=
 =?us-ascii?Q?XoP/wDdGN3l+ImPy/y9+OYuqYl9FK0B4Cz19lcv8fnnHlCyPJX0gy4nzAEib?=
 =?us-ascii?Q?PduYQiMfGyuVhK6Vj57OwF0NnsSwe8x38EkmibTbcjtkCT6/o9QsbGcq7dvf?=
 =?us-ascii?Q?8DY/pCoZEpe4lTW6Ocg6D5M3f1x8GhHlLAdXRTosNJAbgI53OXm1/uWPm2C+?=
 =?us-ascii?Q?GfTMypS2crciKQL5LUTixLf/hUr6qZoDbmwahU4l/SJybB+33plqaKEZt0Ib?=
 =?us-ascii?Q?V59VIwO6F+05MMUWcQNz0PnJV0je21NFmOgchwPAxIKk8QbUrukeD4RhTHQH?=
 =?us-ascii?Q?JgMjzselFqfcg2byUrYoCJ1+A+InEB2QrHwJRCnC5XvnKBfSYzTRNw2ZSpmc?=
 =?us-ascii?Q?JVMhbvR/RLbSia+G2mC07Bz4zTni7XHsEIDdLjcD7eF7466cuggX1EVco8um?=
 =?us-ascii?Q?T3zR3PjbLlRMXNNzHh1yULWLen+wm3W2t2yix0jMu+RhOciHUQUJ8M3C4qrQ?=
 =?us-ascii?Q?TtMKsz83qEcETtkMJUVUpafyDiPEvTa4AdK863l8?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?Y5Y5YL7Av8I0sfhlHKWtIL184Vg7t4h+EITufpkDGsnqR/kouCKQr35r1+Zw?=
 =?us-ascii?Q?Gs4cyZglQhXPB4vCo8DvNARnUdfJOAcXqEqoAG/hb3ns8Ats+Llshc8dSqsa?=
 =?us-ascii?Q?5WpOZpB6MT0ZZScck6iEo3sb1ncBhNZ/mN9car6kBcje5bhD8EIVpHX1RNQz?=
 =?us-ascii?Q?Uo6aFm/Ot6e9obVmKSJGzKwcytHulftUXHJhvb08qFJhBr8+ujLBOxQ6BN04?=
 =?us-ascii?Q?rr01s028P9Z21QgjgUyHL0yFk0l+JLaMwc35TGDjvLPXfq2TZ0YFvenf0/Zc?=
 =?us-ascii?Q?qrrwGPmhG/nEGg0IWiYTgw05Rpb6TpE/RENw3Bx2QlxfVEHEwDWOIYuTY/5i?=
 =?us-ascii?Q?YBoe8j/TD7iDAdT8BpiLM17yhwaefOazR38eVZm/2ELZXv+XFM+Fgczc3KNy?=
 =?us-ascii?Q?f2qFN1/BYydUbWwz57Z9NLAjxGb1HJeD2gz/QIXNyuODyCLB8KggXes+/TJ6?=
 =?us-ascii?Q?MOwtT7bvj77R9Z2gNlyiTx3snud7D50a0qoF+DYFyoPhdZQcTxcWIb2R1Q9C?=
 =?us-ascii?Q?OHhHT6jPaSES7TpiOm0Tw6spYvaPrmic6Wux7RXUC1Y1iHajabEWZF6Yrf9U?=
 =?us-ascii?Q?1Gpulytn1ipMOypvIw6AaZ/0j0Jk5/NIi6jxjns635Qt+NiSSPP+hA8rIOqP?=
 =?us-ascii?Q?KJb0UL4h5OTH+g5cedkTHEiwml8QaHweLnn05AovNoCjUFikdIE8gxGiTnvK?=
 =?us-ascii?Q?flb4kJHwg7JOOr3KV5Tfn4lWdasn5JrU0QGuU4g5kS7kq+uwC69DLwZ7capK?=
 =?us-ascii?Q?wUb4O+4PSgHxAlPUsiCfKbyoooDQrco15OlIJvjZzaccU6zLlLpVmOk1xQIC?=
 =?us-ascii?Q?fJrzvv9MXXsI49DGFzIet6RcotTs8RAY+Uje2YmgUsjOOzL3Vwh/6O7aGrRv?=
 =?us-ascii?Q?2azDFgbg2oLh/Yu8wJr8p+ZsWA2AnexZKeXJ2CFFrtefDfOiThpdpgutfeYq?=
 =?us-ascii?Q?a2X8qgtNyl6N8LqZDOz7uankIUwLocz9tHnA2X9kxNGwUTs/PVUFfUBV66ll?=
 =?us-ascii?Q?ZlYVwbnqCpKaG5U/Dbyy64EWhok5xQezPyJUVg63lrtrPlBTK+FzCAk93JBU?=
 =?us-ascii?Q?7TjXTrGQJXi9DBdsTxEAvWOoRS6ob1lEw7PKFA+0HToUDbQCs/V6UJujhj+g?=
 =?us-ascii?Q?vk1OtkK0mCLlTgBaLwJWjzHbZI+sn7XN2fcG0v+4OZXY9Jw+xUFgKWKS49VK?=
 =?us-ascii?Q?3XykaHPf0nfFGvrv2inLjkAmvluzBH77yAiHtgQxL+8SDBcC6TUdZ7wLFyZo?=
 =?us-ascii?Q?S7FCAG/F7y03bjqnL8N2Xl8u1azj+uqsO/tXvSISnA=3D=3D?=
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR04MB6575.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 581ca989-b3db-44f5-6a02-08dacc61bbcb
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Nov 2022 08:15:36.0441
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: BLdyu6DeHImT6G2VCkGvBYaZL3alW1hLXgE81/J0S/T600Up1qH8R4/vDGT3m+guE7n7or/0lJL2gp3Livuxcw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR04MB5229
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Bean Huo <beanhuo@micron.com>
>=20
> Take out the "map scatter-gather list to prdt" part of the code in
> ufshcd_map_sg and split it into a new function ufshcd_sgl_to_prdt.
>=20
> Signed-off-by: Bean Huo <beanhuo@micron.com>
A nit below.

Reviewed-by: Avri Altman <avri.altman@wdc.com>

> ---
>  drivers/ufs/core/ufshcd.c | 50 ++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
>=20
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> 768cb49d269c..1b252e6cf93f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -2399,38 +2399,30 @@ int ufshcd_send_uic_cmd(struct ufs_hba *hba,
> struct uic_command *uic_cmd)  }
>=20
>  /**
> - * ufshcd_map_sg - Map scatter-gather list to prdt
> - * @hba: per adapter instance
> - * @lrbp: pointer to local reference block
> - *
> - * Returns 0 in case of success, non-zero value in case of failure
> + * ufshcd_sgl_to_prdt - SG list to PRTD (Physical Region Description Tab=
le,
> 4DW format)
> + * @hba:       per-adapter instance
> + * @lrbp:      pointer to local reference block
> + * @sg_entries:        The number of sg lists actually used
> + * @sg_list:   Pointer to SG list
>   */
> -static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> +static void ufshcd_sgl_to_prdt(struct ufs_hba *hba, struct ufshcd_lrb *l=
rbp,
> int sg_entries,
> +                              struct scatterlist *sg_list)
>  {
>         struct ufshcd_sg_entry *prd_table;
>         struct scatterlist *sg;
> -       struct scsi_cmnd *cmd;
> -       int sg_segments;
>         int i;
>=20
> -       cmd =3D lrbp->cmd;
> -       sg_segments =3D scsi_dma_map(cmd);
> -       if (sg_segments < 0)
> -               return sg_segments;
> -
> -       if (sg_segments) {
> +       if (sg_entries) {
>=20
>                 if (hba->quirks & UFSHCD_QUIRK_PRDT_BYTE_GRAN)
>                         lrbp->utr_descriptor_ptr->prd_table_length =3D
> -                               cpu_to_le16((sg_segments *
> -                                       sizeof(struct ufshcd_sg_entry)));
> +                               cpu_to_le16((sg_entries * sizeof(struct
> + ufshcd_sg_entry)));
>                 else
> -                       lrbp->utr_descriptor_ptr->prd_table_length =3D
> -                               cpu_to_le16(sg_segments);
> +                       lrbp->utr_descriptor_ptr->prd_table_length =3D
> + cpu_to_le16(sg_entries);
>=20
>                 prd_table =3D lrbp->ucd_prdt_ptr;
>=20
> -               scsi_for_each_sg(cmd, sg, sg_segments, i) {
> +               for_each_sg(sg_list, sg, sg_entries, i) {
>                         const unsigned int len =3D sg_dma_len(sg);
>=20
>                         /*
> @@ -2449,6 +2441,26 @@ static int ufshcd_map_sg(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
>         } else {
>                 lrbp->utr_descriptor_ptr->prd_table_length =3D 0;
>         }
> +}
> +
> +/**
> + * ufshcd_map_sg - Map scatter-gather list to prdt
> + * @hba: per adapter instance
> + * @lrbp: pointer to local reference block
> + *
> + * Returns 0 in case of success, non-zero value in case of failure  */
> +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
> +{
> +       struct scsi_cmnd *cmd;
> +       int sg_segments;
> +
> +       cmd =3D lrbp->cmd;
> +       sg_segments =3D scsi_dma_map(cmd);
Maybe initialize in declaration?

> +       if (sg_segments < 0)
> +               return sg_segments;
> +
> +       ufshcd_sgl_to_prdt(hba, lrbp, sg_segments, scsi_sglist(cmd));
>=20
>         return 0;
>  }
> --
> 2.25.1