| Message ID | 20200221174754.5295-4-michael@walle.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty: serial: fsl_lpuart various fixes and LS1028A support | expand |
On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: > > The DMA channel might not be available at the first probe time. This is > esp. the case if the DMA controller has an IOMMU mapping. > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also > reorder the code a bit, so that we don't prepare the whole UART just to > determine that the DMA channel is not ready yet and we have to undo all > the stuff. Try to map the DMA channels earlier. Changing this means you never probe successfully if you boot a kernel with the DMA driver disabled (or it's IOMMU disabled). Some other drivers request DMA in open() and can work either way. Rob
On Thu, Feb 27, 2020 at 4:35 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: > > > > The DMA channel might not be available at the first probe time. This is > > esp. the case if the DMA controller has an IOMMU mapping. > > > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also > > reorder the code a bit, so that we don't prepare the whole UART just to > > determine that the DMA channel is not ready yet and we have to undo all > > the stuff. Try to map the DMA channels earlier. > > Changing this means you never probe successfully if you boot a kernel > with the DMA driver disabled (or it's IOMMU disabled). Some other > drivers request DMA in open() and can work either way. We got this exact issue previously with another driver. When the required DMA driver is disabled, the DMA framework cannot figure out this situation and keeps returning EPROBE_DEFER. I'm wondering if we should update the DMA framework to use your deferred probe timeout mechanism. Is it still only used for debug purpose? Regards, Leo
On Thu, Feb 27, 2020 at 4:49 PM Li Yang <leoyang.li@nxp.com> wrote: > > On Thu, Feb 27, 2020 at 4:35 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: > > > > > > The DMA channel might not be available at the first probe time. This is > > > esp. the case if the DMA controller has an IOMMU mapping. > > > > > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also > > > reorder the code a bit, so that we don't prepare the whole UART just to > > > determine that the DMA channel is not ready yet and we have to undo all > > > the stuff. Try to map the DMA channels earlier. > > > > Changing this means you never probe successfully if you boot a kernel > > with the DMA driver disabled (or it's IOMMU disabled). Some other > > drivers request DMA in open() and can work either way. > > We got this exact issue previously with another driver. When the > required DMA driver is disabled, the DMA framework cannot figure out > this situation and keeps returning EPROBE_DEFER. I'm wondering if we > should update the DMA framework to use your deferred probe timeout > mechanism. Is it still only used for debug purpose? It's undergoing some rework ATM to not just be for debug. However, it's not really going to help you if you care about the console because waiting for the timeout will be too late to register the console. Rob
Hi Rob, Hi Leo, Am 2020-02-28 00:03, schrieb Rob Herring: > On Thu, Feb 27, 2020 at 4:49 PM Li Yang <leoyang.li@nxp.com> wrote: >> >> On Thu, Feb 27, 2020 at 4:35 PM Rob Herring <robh+dt@kernel.org> >> wrote: >> > >> > On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: >> > > >> > > The DMA channel might not be available at the first probe time. This is >> > > esp. the case if the DMA controller has an IOMMU mapping. >> > > >> > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also >> > > reorder the code a bit, so that we don't prepare the whole UART just to >> > > determine that the DMA channel is not ready yet and we have to undo all >> > > the stuff. Try to map the DMA channels earlier. >> > >> > Changing this means you never probe successfully if you boot a kernel >> > with the DMA driver disabled (or it's IOMMU disabled). Some other >> > drivers request DMA in open() and can work either way. Oh, I see. >> We got this exact issue previously with another driver. When the What driver is it? I've been working on the i2c-mxs.c driver which has the same problem. Ie. its not working with DMA when the IOMMU is enabled. Now that I've learned that dma_request_chan() will return EPROBE_DEFER if the actual DMA driver is not available, I don't think there is any trick like this there. There is no function which would be called late except you'd do something like on the first master_xfer() try to request the DMA channels. But I don't think that would be the way to go. -michael >> required DMA driver is disabled, the DMA framework cannot figure out >> this situation and keeps returning EPROBE_DEFER. I'm wondering if we >> should update the DMA framework to use your deferred probe timeout >> mechanism. Is it still only used for debug purpose? > > It's undergoing some rework ATM to not just be for debug. However, > it's not really going to help you if you care about the console > because waiting for the timeout will be too late to register the > console.
Am 2020-02-28 12:46, schrieb Michael Walle: > Hi Rob, Hi Leo, > > Am 2020-02-28 00:03, schrieb Rob Herring: >> On Thu, Feb 27, 2020 at 4:49 PM Li Yang <leoyang.li@nxp.com> wrote: >>> >>> On Thu, Feb 27, 2020 at 4:35 PM Rob Herring <robh+dt@kernel.org> >>> wrote: >>> > >>> > On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: >>> > > >>> > > The DMA channel might not be available at the first probe time. This is >>> > > esp. the case if the DMA controller has an IOMMU mapping. >>> > > >>> > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also >>> > > reorder the code a bit, so that we don't prepare the whole UART just to >>> > > determine that the DMA channel is not ready yet and we have to undo all >>> > > the stuff. Try to map the DMA channels earlier. >>> > >>> > Changing this means you never probe successfully if you boot a kernel >>> > with the DMA driver disabled (or it's IOMMU disabled). Some other >>> > drivers request DMA in open() and can work either way. > > Oh, I see. > >>> We got this exact issue previously with another driver. When the > > What driver is it? I've been working on the i2c-mxs.c driver which has whoops, i2c-imx.c, not i2c-mxs.c -michael > the same problem. Ie. its not working with DMA when the IOMMU is > enabled. > Now that I've learned that dma_request_chan() will return EPROBE_DEFER > if the actual DMA driver is not available, I don't think there is any > trick like this there. There is no function which would be called late > except you'd do something like on the first master_xfer() try to > request > the DMA channels. But I don't think that would be the way to go. > > -michael > >>> required DMA driver is disabled, the DMA framework cannot figure out >>> this situation and keeps returning EPROBE_DEFER. I'm wondering if we >>> should update the DMA framework to use your deferred probe timeout >>> mechanism. Is it still only used for debug purpose? >> >> It's undergoing some rework ATM to not just be for debug. However, >> it's not really going to help you if you care about the console >> because waiting for the timeout will be too late to register the >> console.
On Fri, Feb 28, 2020 at 3:51 PM Michael Walle <michael@walle.cc> wrote: > > Am 2020-02-28 12:46, schrieb Michael Walle: > > Hi Rob, Hi Leo, > > > > Am 2020-02-28 00:03, schrieb Rob Herring: > >> On Thu, Feb 27, 2020 at 4:49 PM Li Yang <leoyang.li@nxp.com> wrote: > >>> > >>> On Thu, Feb 27, 2020 at 4:35 PM Rob Herring <robh+dt@kernel.org> > >>> wrote: > >>> > > >>> > On Fri, Feb 21, 2020 at 11:48 AM Michael Walle <michael@walle.cc> wrote: > >>> > > > >>> > > The DMA channel might not be available at the first probe time. This is > >>> > > esp. the case if the DMA controller has an IOMMU mapping. > >>> > > > >>> > > Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also > >>> > > reorder the code a bit, so that we don't prepare the whole UART just to > >>> > > determine that the DMA channel is not ready yet and we have to undo all > >>> > > the stuff. Try to map the DMA channels earlier. > >>> > > >>> > Changing this means you never probe successfully if you boot a kernel > >>> > with the DMA driver disabled (or it's IOMMU disabled). Some other > >>> > drivers request DMA in open() and can work either way. > > > > Oh, I see. > > > >>> We got this exact issue previously with another driver. When the > > > > What driver is it? I've been working on the i2c-mxs.c driver which has > > whoops, i2c-imx.c, not i2c-mxs.c Ya. Same one. I have added CONFIG_FSL_EDMA into the arm64/defconfig as a workaround. Regards, Leo
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c index c31b8f3db6bf..fd9f60d0817a 100644 --- a/drivers/tty/serial/fsl_lpuart.c +++ b/drivers/tty/serial/fsl_lpuart.c @@ -2416,6 +2416,7 @@ static int lpuart_probe(struct platform_device *pdev) const struct lpuart_soc_data *sdata = of_id->data; struct device_node *np = pdev->dev.of_node; struct lpuart_port *sport; + struct dma_chan *dma_chan; struct resource *res; int ret; @@ -2483,6 +2484,26 @@ static int lpuart_probe(struct platform_device *pdev) } sport->port.line = ret; + dma_chan = dma_request_chan(sport->port.dev, "tx"); + if (PTR_ERR(dma_chan) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto failed_request_tx_dma; + } else if (IS_ERR(dma_chan)) + dev_info(sport->port.dev, "DMA tx channel request failed, " + "operating without tx DMA\n"); + else + sport->dma_tx_chan = dma_chan; + + dma_chan = dma_request_chan(sport->port.dev, "rx"); + if (PTR_ERR(dma_chan) == -EPROBE_DEFER) { + ret = -EPROBE_DEFER; + goto failed_request_rx_dma; + } else if (IS_ERR(dma_chan)) + dev_info(sport->port.dev, "DMA rx channel request failed, " + "operating without rx DMA\n"); + else + sport->dma_rx_chan = dma_chan; + ret = lpuart_enable_clks(sport); if (ret) goto failed_clock_enable; @@ -2520,22 +2541,16 @@ static int lpuart_probe(struct platform_device *pdev) sport->port.rs485_config(&sport->port, &sport->port.rs485); - sport->dma_tx_chan = dma_request_slave_channel(sport->port.dev, "tx"); - if (!sport->dma_tx_chan) - dev_info(sport->port.dev, "DMA tx channel request failed, " - "operating without tx DMA\n"); - - sport->dma_rx_chan = dma_request_slave_channel(sport->port.dev, "rx"); - if (!sport->dma_rx_chan) - dev_info(sport->port.dev, "DMA rx channel request failed, " - "operating without rx DMA\n"); - return 0; failed_attach_port: failed_irq_request: lpuart_disable_clks(sport); failed_clock_enable: + dma_release_channel(sport->dma_rx_chan); +failed_request_rx_dma: + dma_release_channel(sport->dma_tx_chan); +failed_request_tx_dma: failed_out_of_range: if (sport->id_allocated) ida_simple_remove(&fsl_lpuart_ida, sport->port.line);
The DMA channel might not be available at the first probe time. This is esp. the case if the DMA controller has an IOMMU mapping. Use the new dma_request_chan() API and handle EPROBE_DEFER errors. Also reorder the code a bit, so that we don't prepare the whole UART just to determine that the DMA channel is not ready yet and we have to undo all the stuff. Try to map the DMA channels earlier. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/tty/serial/fsl_lpuart.c | 35 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-)