Messages in this thread | | | Subject | Re: [PATCH V2] dma: dw-edma-pcie: switch from 'pci_' to 'dma_' API | From | Christophe JAILLET <> | Date | Tue, 9 Nov 2021 22:11:52 +0100 |
| |
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 > >
| |