[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e8e01508-fe2d-a9f8-c260-fb8c6d7bcdcb@huawei.com>
Date: Sat, 24 Sep 2022 10:54:57 +0800
From: Ruan Jinjie <ruanjinjie@...wei.com>
To: Franky Lin <franky.lin@...adcom.com>
CC: <aspriel@...il.com>, <hante.meuleman@...adcom.com>,
<kvalo@...nel.org>, <davem@...emloft.net>, <edumazet@...gle.com>,
<kuba@...nel.org>, <pabeni@...hat.com>, <marcan@...can.st>,
<linus.walleij@...aro.org>, <rmk+kernel@...linux.org.uk>,
<soontak.lee@...ress.com>, <linux-wireless@...r.kernel.org>,
<SHA-cyfmac-dev-list@...ineon.com>,
<brcm80211-dev-list.pdl@...adcom.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next] wifi: brcmfmac: pcie: add missing
pci_disable_device() in brcmf_pcie_get_resource()
On 2022/9/24 0:50, Franky Lin wrote:
> On Fri, Sep 23, 2022 at 2:42 AM ruanjinjie <ruanjinjie@...wei.com> wrote:
>>
>> Add missing pci_disable_device() if brcmf_pcie_get_resource() fails.
>
> Did you encounter any issue because of the absensent
> pci_disable_device? A bit more context will be very helpful.
>
>>
>> Signed-off-by: ruanjinjie <ruanjinjie@...wei.com>
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> index f98641bb1528..25fa69793d86 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> @@ -1725,7 +1725,8 @@ static int brcmf_pcie_get_resource(struct brcmf_pciedev_info *devinfo)
>> if ((bar1_size == 0) || (bar1_addr == 0)) {
>> brcmf_err(bus, "BAR1 Not enabled, device size=%ld, addr=%#016llx\n",
>> bar1_size, (unsigned long long)bar1_addr);
>> - return -EINVAL;
>> + err = -EINVAL;
>> + goto err_disable;
>> }
>>
>> devinfo->regs = ioremap(bar0_addr, BRCMF_PCIE_REG_MAP_SIZE);
>> @@ -1734,7 +1735,8 @@ static int brcmf_pcie_get_resource(struct brcmf_pciedev_info *devinfo)
>> if (!devinfo->regs || !devinfo->tcm) {
>> brcmf_err(bus, "ioremap() failed (%p,%p)\n", devinfo->regs,
>> devinfo->tcm);
>> - return -EINVAL;
>> + err = -EINVAL;
>> + goto err_disable;
>> }
>> brcmf_dbg(PCIE, "Phys addr : reg space = %p base addr %#016llx\n",
>> devinfo->regs, (unsigned long long)bar0_addr);
>> @@ -1743,6 +1745,9 @@ static int brcmf_pcie_get_resource(struct brcmf_pciedev_info *devinfo)
>> (unsigned int)bar1_size);
>>
>> return 0;
>> +err_disable:
>> + pci_disable_device(pdev);
>
> Isn't brcmf_pcie_release_resource() a better choice which also unmap
> the io if any was mapped?
>
That is a better choice to call pci_disable_device() in
brcmf_pcie_release_resource()!
> Regards,
> - Franky
>
>> + return err;
>> }
>>
>>
>> --
>> 2.25.1
>>
Powered by blists - more mailing lists