| Message ID | 20221110015034.7943-4-lihuisong@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ACPI: PCC: optimize pcc codes and fix one bug | expand |
On Thu, Nov 10, 2022 at 09:50:34AM +0800, Huisong Li wrote: > Currently, 'pcc_chan_count' is a non-zero value if PCC subspaces are parsed > successfully and subsequent processes is failure during initializing PCC > process. This may cause that pcc_mbox_request_channel() can still be > executed successfully , which will misleads the caller that this channel is > available. > > Fixes: ce028702ddbc ("mailbox: pcc: Move bulk of PCCT parsing into pcc_mbox_probe") > Signed-off-by: Huisong Li <lihuisong@huawei.com> > --- > drivers/mailbox/pcc.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 7cee37dd3b73..47d70c5884e3 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -294,6 +294,7 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id) > pr_err("Channel not found for idx: %d\n", subspace_id); > return ERR_PTR(-EBUSY); > } > + Spurious/not needed change ? > dev = chan->mbox->dev; > > spin_lock_irqsave(&chan->lock, flags); > @@ -735,7 +736,8 @@ static int __init pcc_init(void) > > if (ret) { > pr_debug("ACPI PCC probe failed.\n"); > - return -ENODEV; > + ret = -ENODEV; > + goto out; Not needed, we don't set pcc_chan_count if the probe failed. > } > > pcc_pdev = platform_create_bundle(&pcc_mbox_driver, > @@ -743,10 +745,13 @@ static int __init pcc_init(void) > > if (IS_ERR(pcc_pdev)) { > pr_debug("Err creating PCC platform bundle\n"); > - return PTR_ERR(pcc_pdev); > + ret = PTR_ERR(pcc_pdev); You just need to set pcc_chan_count to 0 here, so no need for goto.
在 2022/11/10 18:44, Sudeep Holla 写道: > On Thu, Nov 10, 2022 at 09:50:34AM +0800, Huisong Li wrote: >> Currently, 'pcc_chan_count' is a non-zero value if PCC subspaces are parsed >> successfully and subsequent processes is failure during initializing PCC >> process. This may cause that pcc_mbox_request_channel() can still be >> executed successfully , which will misleads the caller that this channel is >> available. >> >> Fixes: ce028702ddbc ("mailbox: pcc: Move bulk of PCCT parsing into pcc_mbox_probe") >> Signed-off-by: Huisong Li <lihuisong@huawei.com> >> --- >> drivers/mailbox/pcc.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >> index 7cee37dd3b73..47d70c5884e3 100644 >> --- a/drivers/mailbox/pcc.c >> +++ b/drivers/mailbox/pcc.c >> @@ -294,6 +294,7 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id) >> pr_err("Channel not found for idx: %d\n", subspace_id); >> return ERR_PTR(-EBUSY); >> } >> + > Spurious/not needed change ? Ack > >> dev = chan->mbox->dev; >> >> spin_lock_irqsave(&chan->lock, flags); >> @@ -735,7 +736,8 @@ static int __init pcc_init(void) >> >> if (ret) { >> pr_debug("ACPI PCC probe failed.\n"); >> - return -ENODEV; >> + ret = -ENODEV; >> + goto out; > Not needed, we don't set pcc_chan_count if the probe failed. You are right. will fix it in v2, thanks. > >> } >> >> pcc_pdev = platform_create_bundle(&pcc_mbox_driver, >> @@ -743,10 +745,13 @@ static int __init pcc_init(void) >> >> if (IS_ERR(pcc_pdev)) { >> pr_debug("Err creating PCC platform bundle\n"); >> - return PTR_ERR(pcc_pdev); >> + ret = PTR_ERR(pcc_pdev); > You just need to set pcc_chan_count to 0 here, so no need for goto. Ack >
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index 7cee37dd3b73..47d70c5884e3 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -294,6 +294,7 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id) pr_err("Channel not found for idx: %d\n", subspace_id); return ERR_PTR(-EBUSY); } + dev = chan->mbox->dev; spin_lock_irqsave(&chan->lock, flags); @@ -735,7 +736,8 @@ static int __init pcc_init(void) if (ret) { pr_debug("ACPI PCC probe failed.\n"); - return -ENODEV; + ret = -ENODEV; + goto out; } pcc_pdev = platform_create_bundle(&pcc_mbox_driver, @@ -743,10 +745,13 @@ static int __init pcc_init(void) if (IS_ERR(pcc_pdev)) { pr_debug("Err creating PCC platform bundle\n"); - return PTR_ERR(pcc_pdev); + ret = PTR_ERR(pcc_pdev); + goto out; } - return 0; +out: + pcc_chan_count = 0; + return ret; } /*
Currently, 'pcc_chan_count' is a non-zero value if PCC subspaces are parsed successfully and subsequent processes is failure during initializing PCC process. This may cause that pcc_mbox_request_channel() can still be executed successfully , which will misleads the caller that this channel is available. Fixes: ce028702ddbc ("mailbox: pcc: Move bulk of PCCT parsing into pcc_mbox_probe") Signed-off-by: Huisong Li <lihuisong@huawei.com> --- drivers/mailbox/pcc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)