Messages in this thread | | | Date | Thu, 02 Aug 2018 10:35:10 +0200 | From | Stefan Agner <> | Subject | Re: [PATCH] net: fec: check DMA addressing limitations |
| |
On 02.08.2018 09:47, Andy Duan wrote: > From: Stefan Agner <stefan@agner.ch> Sent: 2018年8月2日 15:16 >> On 02.08.2018 04:00, Andy Duan wrote: >> > From: Stefan Agner <stefan@agner.ch> Sent: 2018年8月1日 19:45 >> >> Check DMA addressing limitations as suggested by the DMA API how-to. >> >> This does not fix a particular issue seen but is considered good style. >> >> >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> >> --- >> >> drivers/net/ethernet/freescale/fec_main.c | 6 ++++++ >> >> 1 file changed, 6 insertions(+) >> >> >> >> diff --git a/drivers/net/ethernet/freescale/fec_main.c >> >> b/drivers/net/ethernet/freescale/fec_main.c >> >> index c729665107f5..af0fb200e936 100644 >> >> --- a/drivers/net/ethernet/freescale/fec_main.c >> >> +++ b/drivers/net/ethernet/freescale/fec_main.c >> >> @@ -3146,6 +3146,12 @@ static int fec_enet_init(struct net_device >> >> *ndev) >> >> fep->tx_align = 0x3; >> >> #endif >> >> >> >> + /* Check mask of the streaming and coherent API */ >> >> + if (dma_set_mask_and_coherent(&fep->pdev->dev, >> >> DMA_BIT_MASK(32))) { >> >> + dev_warn(&fep->pdev->dev, "No suitable DMA >> available\n"); >> >> + return -ENODEV; >> > It is better: >> > >> > ret = dma_set_mask_and_coherent(&fep->pdev->dev, >> DMA_BIT_MASK(32)); if >> > (ret < 0) { >> > dev_warn(&fep->pdev->dev, "No suitable DMA available\n"); >> > return ret; >> > } >> >> The code comes from the example in >> Documentation/DMA-API-HOWTO.txt. >> >> I can rearrange if you prefer. >> > It is better to return the real error code like -EIO if dma_mask is > not initialized.
Good point. Will send a v2.
-- Stefan
> >> > >> > >> > If the patch aim to "OF: Don't set default coherent DMA mask", I think >> > not only this driver need to add the DMA mask limitations, many other >> > drivers also need. >> >> It doesn't exactly address the issue since with that patch DMA mask wasn't >> set at all. This code would just make the driver fail with -ENODEV instead of >> -ENOMEM. >> > Yes, I agree. It doesn't address the issue if no patch "of/platform: > Initialise default DMA masks". > > For non-DT, it seems dma_coerce_mask_and_coherent() much better. > >> To force a DMA mask I would have to set the DMA mask using >> dma_coerce_mask_and_coherent(). But as discussed with Robin it is safe >> to assume that an initial mask is set by the bus code: >> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flk >> ml.kernel.org%2Fr%2F892f9d14-e6fd-7b1b-d07b-af0be6e623fa%40arm.c >> om&data=02%7C01%7Cfugang.duan%40nxp.com%7C618543fb9b4e >> 4f9fa49908d5f847c47f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% >> 7C0%7C636687909487381876&sdata=hyeeh0KSC3Fu7VDl4A8JbcCO >> %2FwocUpTEMvTv1hjba0E%3D&reserved=0 >> >> Setting the DMA mask on DT buses has been addressed by Robin with >> "of/platform: Initialise default DMA masks". So everything should be fine >> again. >> > Anyway, I am fine with the patch except the return code. > Thanks. > >> I agree, other drivers need fixing too. >> >> -- >> Stefan >> >> > >> > >> >> + } >> >> + >> >> fec_enet_alloc_queue(ndev); >> >> >> >> bd_size = (fep->total_tx_ring_size + fep->total_rx_ring_size) * >> >> dsize; >> >> -- >> >> 2.18.0
| |