* [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API
@ 2021-09-28 3:44 Qing Wang
2021-11-02 19:05 ` Christophe JAILLET
0 siblings, 1 reply; 5+ messages in thread
From: Qing Wang @ 2021-09-28 3:44 UTC (permalink / raw)
To: Gustavo Pimentel, Vinod Koul, dmaengine, linux-kernel; +Cc: Wang Qing
From: Wang Qing <wangqing@vivo.com>
The wrappers in include/linux/pci-dma-compat.h should go away.
The patch has been generated with the coccinelle script below.
expression e1, e2;
@@
- pci_set_dma_mask(e1, e2)
+ dma_set_mask(&e1->dev, e2)
@@
expression e1, e2;
@@
- pci_set_consistent_dma_mask(e1, e2)
+ dma_set_coherent_mask(&e1->dev, e2)
While at it, some 'dma_set_mask()/dma_set_coherent_mask()' have been
updated to a much less verbose 'dma_set_mask_and_coherent()'.
Signed-off-by: Wang Qing <wangqing@vivo.com>
---
drivers/dma/dw-edma/dw-edma-pcie.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 44f6e09..198f6cd
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -186,27 +186,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
pci_set_master(pdev);
/* DMA configuration */
- err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
if (!err) {
- err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
- if (err) {
- pci_err(pdev, "consistent DMA mask 64 set failed\n");
- return err;
- }
+ pci_err(pdev, "DMA mask 64 set failed\n");
+ return err;
} else {
pci_err(pdev, "DMA mask 64 set failed\n");
- err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
if (err) {
pci_err(pdev, "DMA mask 32 set failed\n");
return err;
}
-
- err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
- if (err) {
- pci_err(pdev, "consistent DMA mask 32 set failed\n");
- return err;
- }
}
/* Data structure allocation */
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API
2021-09-28 3:44 [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API Qing Wang
@ 2021-11-02 19:05 ` Christophe JAILLET
2021-11-09 13:21 ` Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Christophe JAILLET @ 2021-11-02 19:05 UTC (permalink / raw)
To: Qing Wang, Gustavo Pimentel, Vinod Koul, dmaengine, linux-kernel,
Kernel Janitors
Hi,
Le 28/09/2021 à 05:44, Qing Wang a écrit :
> From: Wang Qing <wangqing@vivo.com>
>
> The wrappers in include/linux/pci-dma-compat.h should go away.
>
> The patch has been generated with the coccinelle script below.
> expression e1, e2;
> @@
> - pci_set_dma_mask(e1, e2)
> + dma_set_mask(&e1->dev, e2)
>
> @@
> expression e1, e2;
> @@
> - pci_set_consistent_dma_mask(e1, e2)
> + dma_set_coherent_mask(&e1->dev, e2)
>
> While at it, some 'dma_set_mask()/dma_set_coherent_mask()' have been
> updated to a much less verbose 'dma_set_mask_and_coherent()'.
>
> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
> drivers/dma/dw-edma/dw-edma-pcie.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 44f6e09..198f6cd
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -186,27 +186,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> pci_set_master(pdev);
>
> /* DMA configuration */
> - err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> if (!err) {
if err = 0, so if no error...
> - err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
> - if (err) {
> - pci_err(pdev, "consistent DMA mask 64 set failed\n");
> - return err;
> - }
> + pci_err(pdev, "DMA mask 64 set failed\n");
> + return err;
... we log an error, return success but don't perform the last steps of
the probe.
> } else {
> pci_err(pdev, "DMA mask 64 set failed\n");
>
> - err = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> if (err) {
> pci_err(pdev, "DMA mask 32 set failed\n");
> return err;
> }
> -
> - err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
> - if (err) {
> - pci_err(pdev, "consistent DMA mask 32 set failed\n");
> - return err;
> - }
> }
>
> /* Data structure allocation */
>
This patch is broken and should be reworked.
It has been applied in ecb8c88bd31c.
CJ
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API
2021-11-02 19:05 ` Christophe JAILLET
@ 2021-11-09 13:21 ` Dan Carpenter
2021-11-09 13:26 ` Dan Carpenter
2021-11-09 21:11 ` Christophe JAILLET
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-11-09 13:21 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Qing Wang, Gustavo Pimentel, Vinod Koul, dmaengine, linux-kernel,
Kernel Janitors
On Tue, Nov 02, 2021 at 08:05:53PM +0100, Christophe JAILLET wrote:
> Hi,
>
>
> Le 28/09/2021 à 05:44, Qing Wang a écrit :
> > From: Wang Qing <wangqing@vivo.com>
> >
> > The wrappers in include/linux/pci-dma-compat.h should go away.
> >
> > The patch has been generated with the coccinelle script below.
> > expression e1, e2;
> > @@
> > - pci_set_dma_mask(e1, e2)
> > + dma_set_mask(&e1->dev, e2)
> >
> > @@
> > expression e1, e2;
> > @@
> > - pci_set_consistent_dma_mask(e1, e2)
> > + dma_set_coherent_mask(&e1->dev, e2)
> >
> > While at it, some 'dma_set_mask()/dma_set_coherent_mask()' have been
> > updated to a much less verbose 'dma_set_mask_and_coherent()'.
> >
> > Signed-off-by: Wang Qing <wangqing@vivo.com>
> > ---
> > drivers/dma/dw-edma/dw-edma-pcie.c | 17 ++++-------------
> > 1 file changed, 4 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > index 44f6e09..198f6cd
> > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > @@ -186,27 +186,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > pci_set_master(pdev);
> > /* DMA configuration */
> > - err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> > + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> > if (!err) {
> if err = 0, so if no error...
>
> > - err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
> > - if (err) {
> > - pci_err(pdev, "consistent DMA mask 64 set failed\n");
> > - return err;
> > - }
> > + pci_err(pdev, "DMA mask 64 set failed\n");
> > + return err;
> ... we log an error, return success but don't perform the last steps of the
> probe.
I have an unpublished Smatch check for these:
drivers/dma/dw-edma/dw-edma-pcie.c:192 dw_edma_pcie_probe() info: return a literal instead of 'err'
The idea of the Smatch check is that it's pretty easy to get "if (!ret)"
and "if (ret)" transposed. It would show up in testing, of course, but
the truth is that maintainers don't always have all the hardware they
maintain.
And the other idea is that "return 0;" is always more readable and
intentional than "return ret;" where ret is zero.
Anyway, is someone going to fix these?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API
2021-11-09 13:21 ` Dan Carpenter
@ 2021-11-09 13:26 ` Dan Carpenter
2021-11-09 21:11 ` Christophe JAILLET
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-11-09 13:26 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Qing Wang, Gustavo Pimentel, Vinod Koul, dmaengine, linux-kernel,
Kernel Janitors
On Tue, Nov 09, 2021 at 04:21:37PM +0300, Dan Carpenter wrote:
> drivers/dma/dw-edma/dw-edma-pcie.c:192 dw_edma_pcie_probe() info: return a literal instead of 'err'
>
> The idea of the Smatch check is that it's pretty easy to get "if (!ret)"
> and "if (ret)" transposed. It would show up in testing, of course, but
> the truth is that maintainers don't always have all the hardware they
> maintain.
>
> And the other idea is that "return 0;" is always more readable and
> intentional than "return ret;" where ret is zero.
So other style anti-patterns that this check finds are:
net/bluetooth/hci_sync.c
4489
4490 err = hci_start_scan_sync(hdev, LE_SCAN_ACTIVE, interval,
4491 hdev->le_scan_window_discovery,
4492 own_addr_type, filter_policy, filter_dup);
4493 if (!err)
4494 return err;
2) Success handling.
3) Making the last check special/opposite.
*sad face*
4495
4496 failed:
4497 /* Resume advertising if it was paused */
4498 if (use_ll_privacy(hdev))
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API
2021-11-09 13:21 ` Dan Carpenter
2021-11-09 13:26 ` Dan Carpenter
@ 2021-11-09 21:11 ` Christophe JAILLET
1 sibling, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2021-11-09 21:11 UTC (permalink / raw)
To: Dan Carpenter
Cc: Qing Wang, Gustavo Pimentel, Vinod Koul, dmaengine, linux-kernel,
Kernel Janitors
Le 09/11/2021 à 14:21, Dan Carpenter a écrit :
> On Tue, Nov 02, 2021 at 08:05:53PM +0100, Christophe JAILLET wrote:
>> Hi,
>>
>>
>> Le 28/09/2021 à 05:44, Qing Wang a écrit :
>>> From: Wang Qing <wangqing@vivo.com>
>>>
>>> The wrappers in include/linux/pci-dma-compat.h should go away.
>>>
>>> The patch has been generated with the coccinelle script below.
>>> expression e1, e2;
>>> @@
>>> - pci_set_dma_mask(e1, e2)
>>> + dma_set_mask(&e1->dev, e2)
>>>
>>> @@
>>> expression e1, e2;
>>> @@
>>> - pci_set_consistent_dma_mask(e1, e2)
>>> + dma_set_coherent_mask(&e1->dev, e2)
>>>
>>> While at it, some 'dma_set_mask()/dma_set_coherent_mask()' have been
>>> updated to a much less verbose 'dma_set_mask_and_coherent()'.
>>>
>>> Signed-off-by: Wang Qing <wangqing@vivo.com>
>>> ---
>>> drivers/dma/dw-edma/dw-edma-pcie.c | 17 ++++-------------
>>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
>>> index 44f6e09..198f6cd
>>> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
>>> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
>>> @@ -186,27 +186,18 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>>> pci_set_master(pdev);
>>> /* DMA configuration */
>>> - err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
>>> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>>> if (!err) {
>> if err = 0, so if no error...
>>
>>> - err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
>>> - if (err) {
>>> - pci_err(pdev, "consistent DMA mask 64 set failed\n");
>>> - return err;
>>> - }
>>> + pci_err(pdev, "DMA mask 64 set failed\n");
>>> + return err;
>> ... we log an error, return success but don't perform the last steps of the
>> probe.
>
> I have an unpublished Smatch check for these:
>
> drivers/dma/dw-edma/dw-edma-pcie.c:192 dw_edma_pcie_probe() info: return a literal instead of 'err'
>
> The idea of the Smatch check is that it's pretty easy to get "if (!ret)"
> and "if (ret)" transposed. It would show up in testing, of course, but
> the truth is that maintainers don't always have all the hardware they
> maintain.
>
> And the other idea is that "return 0;" is always more readable and
> intentional than "return ret;" where ret is zero.
>
> Anyway, is someone going to fix these?
Patch sent.
Feed-back welcomed.
CJ
>
> regards,
> dan carpenter
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-09 21:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 3:44 [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API Qing Wang
2021-11-02 19:05 ` Christophe JAILLET
2021-11-09 13:21 ` Dan Carpenter
2021-11-09 13:26 ` Dan Carpenter
2021-11-09 21:11 ` Christophe JAILLET
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.