* [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_*
@ 2021-08-14 15:57 Evgeny Novikov
2021-08-14 19:54 ` Andy Shevchenko
2021-08-23 8:12 ` Akihiro TSUKADA
0 siblings, 2 replies; 5+ messages in thread
From: Evgeny Novikov @ 2021-08-14 15:57 UTC (permalink / raw)
To: Akihiro Tsukada
Cc: Evgeny Novikov, Mauro Carvalho Chehab, Andy Shevchenko,
Kirill Shilimanov, linux-media, linux-kernel, ldv-project
pt3_probe() did not free one of IO mappings in case when one of them was
successful while another one failed. The patch fixed that by using
functions pcim_*. Also, it simplifies error handling through switching
to devm_* functions.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
---
v2: Use functions pcim_* and devm_* to simplify code (Andy Shevchenko)
---
drivers/media/pci/pt3/pt3.c | 58 +++++++++----------------------------
1 file changed, 14 insertions(+), 44 deletions(-)
diff --git a/drivers/media/pci/pt3/pt3.c b/drivers/media/pci/pt3/pt3.c
index c0bc86793355..0d51bdf01f43 100644
--- a/drivers/media/pci/pt3/pt3.c
+++ b/drivers/media/pci/pt3/pt3.c
@@ -685,12 +685,6 @@ static void pt3_remove(struct pci_dev *pdev)
for (i = PT3_NUM_FE - 1; i >= 0; i--)
pt3_cleanup_adapter(pt3, i);
i2c_del_adapter(&pt3->i2c_adap);
- kfree(pt3->i2c_buf);
- pci_iounmap(pt3->pdev, pt3->regs[0]);
- pci_iounmap(pt3->pdev, pt3->regs[1]);
- pci_release_regions(pdev);
- pci_disable_device(pdev);
- kfree(pt3);
}
static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
@@ -704,14 +698,14 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (pci_read_config_byte(pdev, PCI_REVISION_ID, &rev) || rev != 1)
return -ENODEV;
- ret = pci_enable_device(pdev);
+ ret = pcim_enable_device(pdev);
if (ret < 0)
return -ENODEV;
pci_set_master(pdev);
- ret = pci_request_regions(pdev, DRV_NAME);
+ ret = pcim_iomap_regions(pdev, BIT(0) | BIT(2), DRV_NAME);
if (ret < 0)
- goto err_disable_device;
+ return ret;
ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
if (ret == 0)
@@ -722,42 +716,32 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
else {
dev_err(&pdev->dev, "Failed to set DMA mask\n");
- goto err_release_regions;
+ return ret;
}
dev_info(&pdev->dev, "Use 32bit DMA\n");
}
- pt3 = kzalloc(sizeof(*pt3), GFP_KERNEL);
- if (!pt3) {
- ret = -ENOMEM;
- goto err_release_regions;
- }
+ pt3 = devm_kzalloc(&pdev->dev, sizeof(*pt3), GFP_KERNEL);
+ if (!pt3)
+ return -ENOMEM;
pci_set_drvdata(pdev, pt3);
pt3->pdev = pdev;
mutex_init(&pt3->lock);
- pt3->regs[0] = pci_ioremap_bar(pdev, 0);
- pt3->regs[1] = pci_ioremap_bar(pdev, 2);
- if (pt3->regs[0] == NULL || pt3->regs[1] == NULL) {
- dev_err(&pdev->dev, "Failed to ioremap\n");
- ret = -ENOMEM;
- goto err_kfree;
- }
+ pt3->regs[0] = pcim_iomap_table(pdev)[0];
+ pt3->regs[1] = pcim_iomap_table(pdev)[2];
ver = ioread32(pt3->regs[0] + REG_VERSION);
if ((ver >> 16) != 0x0301) {
dev_warn(&pdev->dev, "PT%d, I/F-ver.:%d not supported\n",
ver >> 24, (ver & 0x00ff0000) >> 16);
- ret = -ENODEV;
- goto err_iounmap;
+ return -ENODEV;
}
pt3->num_bufs = clamp_val(num_bufs, MIN_DATA_BUFS, MAX_DATA_BUFS);
- pt3->i2c_buf = kmalloc(sizeof(*pt3->i2c_buf), GFP_KERNEL);
- if (pt3->i2c_buf == NULL) {
- ret = -ENOMEM;
- goto err_iounmap;
- }
+ pt3->i2c_buf = devm_kmalloc(&pdev->dev, sizeof(*pt3->i2c_buf), GFP_KERNEL);
+ if (!pt3->i2c_buf)
+ return -ENOMEM;
i2c = &pt3->i2c_adap;
i2c->owner = THIS_MODULE;
i2c->algo = &pt3_i2c_algo;
@@ -767,7 +751,7 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
i2c_set_adapdata(i2c, pt3);
ret = i2c_add_adapter(i2c);
if (ret < 0)
- goto err_i2cbuf;
+ return ret;
for (i = 0; i < PT3_NUM_FE; i++) {
ret = pt3_alloc_adapter(pt3, i);
@@ -799,21 +783,7 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
while (i >= 0)
pt3_cleanup_adapter(pt3, i--);
i2c_del_adapter(i2c);
-err_i2cbuf:
- kfree(pt3->i2c_buf);
-err_iounmap:
- if (pt3->regs[0])
- pci_iounmap(pdev, pt3->regs[0]);
- if (pt3->regs[1])
- pci_iounmap(pdev, pt3->regs[1]);
-err_kfree:
- kfree(pt3);
-err_release_regions:
- pci_release_regions(pdev);
-err_disable_device:
- pci_disable_device(pdev);
return ret;
-
}
static const struct pci_device_id pt3_id_table[] = {
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_*
2021-08-14 15:57 [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_* Evgeny Novikov
@ 2021-08-14 19:54 ` Andy Shevchenko
2021-08-15 1:33 ` Akihiro TSUKADA
2021-08-23 8:12 ` Akihiro TSUKADA
1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2021-08-14 19:54 UTC (permalink / raw)
To: Evgeny Novikov
Cc: Akihiro Tsukada, Mauro Carvalho Chehab, Kirill Shilimanov,
Linux Media Mailing List, Linux Kernel Mailing List, ldv-project
On Sat, Aug 14, 2021 at 6:57 PM Evgeny Novikov <novikov@ispras.ru> wrote:
>
> pt3_probe() did not free one of IO mappings in case when one of them was
> successful while another one failed. The patch fixed that by using
> functions pcim_*. Also, it simplifies error handling through switching
> to devm_* functions.
>
> Found by Linux Driver Verification project (linuxtesting.org).
Looks good to me, though I can't test this myself either (I have no hardware).
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Evgeny Novikov <novikov@ispras.ru>
> Co-developed-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> Signed-off-by: Kirill Shilimanov <kirill.shilimanov@huawei.com>
> ---
> v2: Use functions pcim_* and devm_* to simplify code (Andy Shevchenko)
> ---
> drivers/media/pci/pt3/pt3.c | 58 +++++++++----------------------------
> 1 file changed, 14 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/media/pci/pt3/pt3.c b/drivers/media/pci/pt3/pt3.c
> index c0bc86793355..0d51bdf01f43 100644
> --- a/drivers/media/pci/pt3/pt3.c
> +++ b/drivers/media/pci/pt3/pt3.c
> @@ -685,12 +685,6 @@ static void pt3_remove(struct pci_dev *pdev)
> for (i = PT3_NUM_FE - 1; i >= 0; i--)
> pt3_cleanup_adapter(pt3, i);
> i2c_del_adapter(&pt3->i2c_adap);
> - kfree(pt3->i2c_buf);
> - pci_iounmap(pt3->pdev, pt3->regs[0]);
> - pci_iounmap(pt3->pdev, pt3->regs[1]);
> - pci_release_regions(pdev);
> - pci_disable_device(pdev);
> - kfree(pt3);
> }
>
> static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> @@ -704,14 +698,14 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (pci_read_config_byte(pdev, PCI_REVISION_ID, &rev) || rev != 1)
> return -ENODEV;
>
> - ret = pci_enable_device(pdev);
> + ret = pcim_enable_device(pdev);
> if (ret < 0)
> return -ENODEV;
> pci_set_master(pdev);
>
> - ret = pci_request_regions(pdev, DRV_NAME);
> + ret = pcim_iomap_regions(pdev, BIT(0) | BIT(2), DRV_NAME);
> if (ret < 0)
> - goto err_disable_device;
> + return ret;
>
> ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
> if (ret == 0)
> @@ -722,42 +716,32 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> else {
> dev_err(&pdev->dev, "Failed to set DMA mask\n");
> - goto err_release_regions;
> + return ret;
> }
> dev_info(&pdev->dev, "Use 32bit DMA\n");
> }
>
> - pt3 = kzalloc(sizeof(*pt3), GFP_KERNEL);
> - if (!pt3) {
> - ret = -ENOMEM;
> - goto err_release_regions;
> - }
> + pt3 = devm_kzalloc(&pdev->dev, sizeof(*pt3), GFP_KERNEL);
> + if (!pt3)
> + return -ENOMEM;
> pci_set_drvdata(pdev, pt3);
> pt3->pdev = pdev;
> mutex_init(&pt3->lock);
> - pt3->regs[0] = pci_ioremap_bar(pdev, 0);
> - pt3->regs[1] = pci_ioremap_bar(pdev, 2);
> - if (pt3->regs[0] == NULL || pt3->regs[1] == NULL) {
> - dev_err(&pdev->dev, "Failed to ioremap\n");
> - ret = -ENOMEM;
> - goto err_kfree;
> - }
> + pt3->regs[0] = pcim_iomap_table(pdev)[0];
> + pt3->regs[1] = pcim_iomap_table(pdev)[2];
>
> ver = ioread32(pt3->regs[0] + REG_VERSION);
> if ((ver >> 16) != 0x0301) {
> dev_warn(&pdev->dev, "PT%d, I/F-ver.:%d not supported\n",
> ver >> 24, (ver & 0x00ff0000) >> 16);
> - ret = -ENODEV;
> - goto err_iounmap;
> + return -ENODEV;
> }
>
> pt3->num_bufs = clamp_val(num_bufs, MIN_DATA_BUFS, MAX_DATA_BUFS);
>
> - pt3->i2c_buf = kmalloc(sizeof(*pt3->i2c_buf), GFP_KERNEL);
> - if (pt3->i2c_buf == NULL) {
> - ret = -ENOMEM;
> - goto err_iounmap;
> - }
> + pt3->i2c_buf = devm_kmalloc(&pdev->dev, sizeof(*pt3->i2c_buf), GFP_KERNEL);
> + if (!pt3->i2c_buf)
> + return -ENOMEM;
> i2c = &pt3->i2c_adap;
> i2c->owner = THIS_MODULE;
> i2c->algo = &pt3_i2c_algo;
> @@ -767,7 +751,7 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> i2c_set_adapdata(i2c, pt3);
> ret = i2c_add_adapter(i2c);
> if (ret < 0)
> - goto err_i2cbuf;
> + return ret;
>
> for (i = 0; i < PT3_NUM_FE; i++) {
> ret = pt3_alloc_adapter(pt3, i);
> @@ -799,21 +783,7 @@ static int pt3_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> while (i >= 0)
> pt3_cleanup_adapter(pt3, i--);
> i2c_del_adapter(i2c);
> -err_i2cbuf:
> - kfree(pt3->i2c_buf);
> -err_iounmap:
> - if (pt3->regs[0])
> - pci_iounmap(pdev, pt3->regs[0]);
> - if (pt3->regs[1])
> - pci_iounmap(pdev, pt3->regs[1]);
> -err_kfree:
> - kfree(pt3);
> -err_release_regions:
> - pci_release_regions(pdev);
> -err_disable_device:
> - pci_disable_device(pdev);
> return ret;
> -
> }
>
> static const struct pci_device_id pt3_id_table[] = {
> --
> 2.26.2
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_*
2021-08-14 19:54 ` Andy Shevchenko
@ 2021-08-15 1:33 ` Akihiro TSUKADA
0 siblings, 0 replies; 5+ messages in thread
From: Akihiro TSUKADA @ 2021-08-15 1:33 UTC (permalink / raw)
To: Andy Shevchenko, Evgeny Novikov
Cc: Mauro Carvalho Chehab, Kirill Shilimanov,
Linux Media Mailing List, Linux Kernel Mailing List, ldv-project
I have the device,
but currently I have some hardware problems in my testing environment.
I will test it later when I have a time and fix those problems.
regards,
Akihiro Tsukada
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_*
2021-08-14 15:57 [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_* Evgeny Novikov
2021-08-14 19:54 ` Andy Shevchenko
@ 2021-08-23 8:12 ` Akihiro TSUKADA
2021-08-23 14:40 ` Andy Shevchenko
1 sibling, 1 reply; 5+ messages in thread
From: Akihiro TSUKADA @ 2021-08-23 8:12 UTC (permalink / raw)
To: Evgeny Novikov
Cc: Mauro Carvalho Chehab, Andy Shevchenko, Kirill Shilimanov,
linux-media, linux-kernel, ldv-project
Tested-by: Akihiro Tsukada <tskd08@gmail.com>
--
tsukada
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_*
2021-08-23 8:12 ` Akihiro TSUKADA
@ 2021-08-23 14:40 ` Andy Shevchenko
0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2021-08-23 14:40 UTC (permalink / raw)
To: Akihiro TSUKADA
Cc: Evgeny Novikov, Mauro Carvalho Chehab, Kirill Shilimanov,
linux-media, linux-kernel, ldv-project
On Mon, Aug 23, 2021 at 11:12 AM Akihiro TSUKADA <tskd08@gmail.com> wrote:
>
> Tested-by: Akihiro Tsukada <tskd08@gmail.com>
Thank you very much!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-23 14:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-14 15:57 [PATCH v2] media: pt3: Switch to using functions pcim_* and devm_* Evgeny Novikov
2021-08-14 19:54 ` Andy Shevchenko
2021-08-15 1:33 ` Akihiro TSUKADA
2021-08-23 8:12 ` Akihiro TSUKADA
2021-08-23 14:40 ` Andy Shevchenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.