[<prev] [next>] [day] [month] [year] [list]
Date: Thu, 12 Dec 2019 11:10:15 +0530
From: Vignesh Raghavendra <vigneshr@...com>
To: Can Guo <cang@...eaurora.org>, <asutoshd@...eaurora.org>,
<nguyenb@...eaurora.org>, <rnayak@...eaurora.org>,
<linux-scsi@...r.kernel.org>, <kernel-team@...roid.com>,
<saravanak@...gle.com>, <salyzyn@...gle.com>
CC: Alim Akhtar <alim.akhtar@...sung.com>,
Avri Altman <avri.altman@....com>,
Pedro Sousa <pedrom.sousa@...opsys.com>,
"James E.J. Bottomley" <jejb@...ux.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Evan Green <evgreen@...omium.org>,
Kishon Vijay Abraham I <kishon@...com>,
Stephen Boyd <swboyd@...omium.org>,
Stanley Chu <stanley.chu@...iatek.com>,
Bean Huo <beanhuo@...ron.com>,
Bart Van Assche <bvanassche@....org>,
Venkat Gopalakrishnan <venkatg@...eaurora.org>,
Tomas Winkler <tomas.winkler@...el.com>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Arnd Bergmann <arnd@...db.de>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] scsi: ufs: Modulize ufs-bsg
Hi,
On 11/12/19 2:19 pm, Can Guo wrote:
> In order to improve the flexibility of ufs-bsg, modulizing it is a good
> choice. This change introduces tristate to ufs-bsg to allow users compile
> it as an external module.
>
> Signed-off-by: Can Guo <cang@...eaurora.org>
> ---
[...]
> -int ufs_bsg_probe(struct ufs_hba *hba)
> +static int ufs_bsg_probe(struct ufs_hba *hba)
> {
> - struct device *bsg_dev = &hba->bsg_dev;
> + struct device *bsg_dev;
> struct Scsi_Host *shost = hba->host;
> struct device *parent = &shost->shost_gendev;
> struct request_queue *q;
> int ret;
>
> + bsg_dev = kzalloc(sizeof(*bsg_dev), GFP_KERNEL);
> + if (!bsg_dev)
> + return -ENOMEM;
> +
> + hba->bsg_dev = bsg_dev;
> device_initialize(bsg_dev);
>
> bsg_dev->parent = get_device(parent);
> @@ -217,6 +225,41 @@ int ufs_bsg_probe(struct ufs_hba *hba)
>
> out:
> dev_err(bsg_dev, "fail to initialize a bsg dev %d\n", shost->host_no);
> + hba->bsg_dev = NULL;
Don't we need to free the associated memory before assigning to NULL?
Alternatively can allocation be made with devm_ APIs instead?
> put_device(bsg_dev);
> return ret;
> }
> +
> +static int __init ufs_bsg_init(void)
> +{
> + struct list_head *hba_list = NULL;
> + struct ufs_hba *hba;
> + int ret = 0;
> +
> + ufshcd_get_hba_list_lock(&hba_list);
> + list_for_each_entry(hba, hba_list, list) {
> + ret = ufs_bsg_probe(hba);
> + if (ret)
> + break;
> + }
So IIUC, if ufs_bsg_probe() fails for one of the hba instances in the
list, then we fail to create bsg device for all remaining instances that
follow, which seems too harsh.
Regards
Vignesh
Powered by blists - more mailing lists