RE: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-11 Thread Flavio Suligoi
> > > 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


Re: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-11 Thread Mark Brown
On Thu, Apr 11, 2019 at 07:14:06AM +, 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.


signature.asc
Description: PGP signature


RE: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-11 Thread Flavio Suligoi
> > 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


Re: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-10 Thread Mark Brown
On Wed, Apr 10, 2019 at 02:05:38PM +, 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.


signature.asc
Description: PGP signature


RE: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-10 Thread Flavio Suligoi
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



Re: [PATCH 2/2] spi: pxa2xx: use a module softdep for dw_dmac

2019-04-10 Thread Mark Brown
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?


signature.asc
Description: PGP signature