| Message ID | 1554900696-28858-2-git-send-email-f.suligoi@asem.it (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] spi: pxa2xx: fix SCR (divisor) calculation | expand |
On Wed, Apr 10, 2019 at 02:51:36PM +0200, Flavio Suligoi wrote: > With dw_dmac, sometimes the request of a DMA channel fails because > the DMA driver is not ready, so an explicit dependency request > is necessary. While this isn't going to hurt anything and might actually help so it's fine doesn't this also suggest that there's an issue with deferred probe going on as well?
Hi Mark, > On Wed, Apr 10, 2019 at 02:51:36PM +0200, Flavio Suligoi wrote: > > With dw_dmac, sometimes the request of a DMA channel fails because > > the DMA driver is not ready, so an explicit dependency request > > is necessary. > > While this isn't going to hurt anything and might actually help so it's > fine doesn't this also suggest that there's an issue with deferred probe > going on as well? I think that the problem could be related to how the DMA channel is requested. At the moment the function used are: pxa2xx_spi_dma_setup --> dma_request_slave_channel_compat --> --> __dma_request_slave_channel_compat --> dma_request_slave_channel --> --> dma_request_chan Actually the final function "dma_request_chan" return the channel number or "-EPROBE_DEFER" if it's not ready. But this information ("-EPROBE_DEFER") is lost in the penultimate function "dma_request_slave_channel", which return only the chann, if all is ok, or NULL, in case of errors. So the deferral mechanism is not used. Flavio
On Wed, Apr 10, 2019 at 02:05:38PM +0000, Flavio Suligoi wrote: > Hi Mark, > > While this isn't going to hurt anything and might actually help so it's > > fine doesn't this also suggest that there's an issue with deferred probe > > going on as well? > I think that the problem could be related to how the DMA channel is requested. > At the moment the function used are: > pxa2xx_spi_dma_setup --> dma_request_slave_channel_compat --> > --> __dma_request_slave_channel_compat --> dma_request_slave_channel --> > --> dma_request_chan > Actually the final function "dma_request_chan" return > the channel number or "-EPROBE_DEFER" if it's not ready. > But this information ("-EPROBE_DEFER") is lost in the penultimate function > "dma_request_slave_channel", which return only the chann, if all is ok, or > NULL, in case of errors. > So the deferral mechanism is not used. Right, yes - that analysis seems correct. The interfaces seem a bit weird here but fixing them looks like the most complete and robust fix.
> > I think that the problem could be related to how the DMA channel is > requested. > > At the moment the function used are: > > > pxa2xx_spi_dma_setup --> dma_request_slave_channel_compat --> > > --> __dma_request_slave_channel_compat --> dma_request_slave_channel --> > > --> dma_request_chan > > > Actually the final function "dma_request_chan" return > > the channel number or "-EPROBE_DEFER" if it's not ready. > > But this information ("-EPROBE_DEFER") is lost in the penultimate > function > > "dma_request_slave_channel", which return only the chann, if all is ok, > or > > NULL, in case of errors. > > So the deferral mechanism is not used. > > Right, yes - that analysis seems correct. The interfaces seem a bit > weird here but fixing them looks like the most complete and robust fix. Ok Mark, I'll fix this problem as soon as I can, using EPROBE_DEFER. For now, in my application, I use the patch that I already sent, with the "softdep" workaround: MODULE_SOFTDEP("pre: dw_dmac"); I tested it a lot, with more than 2000 cold reboot (automatic switch on/off using a controlled power supply) and it always worked good. Thanks for your help! Flavio
On Thu, Apr 11, 2019 at 07:14:06AM +0000, Flavio Suligoi wrote: > > Right, yes - that analysis seems correct. The interfaces seem a bit > > weird here but fixing them looks like the most complete and robust fix. > Ok Mark, I'll fix this problem as soon as I can, using EPROBE_DEFER. > For now, in my application, I use the patch that I already sent, > with the "softdep" workaround: > MODULE_SOFTDEP("pre: dw_dmac"); > I tested it a lot, with more than 2000 cold reboot (automatic > switch on/off using a controlled power supply) and it always worked good. Right, and to be clear that patch is good and useful independently of the deferred probe fix so assuming nothing else comes up in review I'll apply it.
> > > Right, yes - that analysis seems correct. The interfaces seem a bit > > > weird here but fixing them looks like the most complete and robust > fix. > > > Ok Mark, I'll fix this problem as soon as I can, using EPROBE_DEFER. > > For now, in my application, I use the patch that I already sent, > > with the "softdep" workaround: > > > MODULE_SOFTDEP("pre: dw_dmac"); > > > I tested it a lot, with more than 2000 cold reboot (automatic > > switch on/off using a controlled power supply) and it always worked > good. > > Right, and to be clear that patch is good and useful independently of > the deferred probe fix so assuming nothing else comes up in review I'll > apply it. Thanks Mark, Flavio
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index c9560a1..e8ac26b 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -1962,3 +1962,5 @@ static void __exit pxa2xx_spi_exit(void) platform_driver_unregister(&driver); } module_exit(pxa2xx_spi_exit); + +MODULE_SOFTDEP("pre: dw_dmac");
With dw_dmac, sometimes the request of a DMA channel fails because the DMA driver is not ready, so an explicit dependency request is necessary. Signed-off-by: Flavio Suligoi <f.suligoi@asem.it> --- drivers/spi/spi-pxa2xx.c | 2 ++ 1 file changed, 2 insertions(+)