Re: Tearing down DMA transfer setup after DMA client has finished

[Mark Brown]

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> On 09/12/2016 18:56, Vinod Koul wrote:

> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.

> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.

> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

Even without the tie into ECC being an issue it seems like falling back
to PIO could produce poor results at the system level - it's likely that
the PIO would be very expensive and take longer than waiting would've
done, increasing the burden on the CPU and slowing things down overall
for userspace.


signature.asc
Description: PGP signature

Re: Tearing down DMA transfer setup after DMA client has finished

[Mark Brown]

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> On 09/12/2016 18:56, Vinod Koul wrote:

> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.

> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.

> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

Even without the tie into ECC being an issue it seems like falling back
to PIO could produce poor results at the system level - it's likely that
the PIO would be very expensive and take longer than waiting would've
done, increasing the burden on the CPU and slowing things down overall
for userspace.


signature.asc
Description: PGP signature

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> [ Dropping Mans to preserve his peace-of-mind ]
> 
> On 09/12/2016 18:56, Vinod Koul wrote:
> > On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> >> On 09/12/2016 18:17, Vinod Koul wrote:
> >>
> >>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> 
>  What concrete solution do you propose?
> >>>
> >>> I have already proposed two solutions.
> >>>
> >>> A) Request a channel only when you need it. Obviously we can't do virtual
> >>> channels with this (though we should still use virt-channels framework).
> >>> The sbox setup and teardown can be done as part of channel request and
> >>> freeup. PL08x already does this.
> >>>
> >>> Downside is that we can only have as many consumers at a time as channels.
> >>>
> >>> I have not heard any technical reason for not doing this apart from 
> >>> drivers
> >>> grab the channel at probe, which is incorrect and needs to be fixed
> >>> irrespective of the problem at hand.
> >>>
> >>> This is my preferred option.
> >>
> >> There is one important drawback with this solution. If a driver calls
> >> dma_request_chan() when no channels are currently available, it will
> >> get -EBUSY. If there were a flag in dma_request_chan to be put to
> >> sleep (with timeout) until a channel is available, then it would
> >> work. But busy waiting in the client driver is a waste of power.
> > 
> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.
> 
> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.
> 
> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

So if you go down this route then we have do a bit of engineering for this
solutions to solve the hardware issue.

Can you tell me the clients for this controller and channels available?

In this case possibly we can dedicate one for NAND and keep the ones with
PIO mode dynamic in nature.. But yes it is not an elegant one.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 07:23:17PM +0100, Mason wrote:
> [ Dropping Mans to preserve his peace-of-mind ]
> 
> On 09/12/2016 18:56, Vinod Koul wrote:
> > On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> >> On 09/12/2016 18:17, Vinod Koul wrote:
> >>
> >>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> 
>  What concrete solution do you propose?
> >>>
> >>> I have already proposed two solutions.
> >>>
> >>> A) Request a channel only when you need it. Obviously we can't do virtual
> >>> channels with this (though we should still use virt-channels framework).
> >>> The sbox setup and teardown can be done as part of channel request and
> >>> freeup. PL08x already does this.
> >>>
> >>> Downside is that we can only have as many consumers at a time as channels.
> >>>
> >>> I have not heard any technical reason for not doing this apart from 
> >>> drivers
> >>> grab the channel at probe, which is incorrect and needs to be fixed
> >>> irrespective of the problem at hand.
> >>>
> >>> This is my preferred option.
> >>
> >> There is one important drawback with this solution. If a driver calls
> >> dma_request_chan() when no channels are currently available, it will
> >> get -EBUSY. If there were a flag in dma_request_chan to be put to
> >> sleep (with timeout) until a channel is available, then it would
> >> work. But busy waiting in the client driver is a waste of power.
> > 
> > Right, but in that case the fallback would be PIO mode, and if that is
> > not availble (IIRC some f your devices don't) then reject the usage with
> > EAGAIN.
> 
> Maybe I'm missing something, but I don't see how that would help.
> Take the NAND Flash controller driver, for instance. PIO is not
> an option, because the ECC engine is tied to DMA.
> 
> And failing with -EAGAIN doesn't help the busy looping situation.
> The caller should be put on some kind of queue to wait for a
> "channel ready" event.

So if you go down this route then we have do a bit of engineering for this
solutions to solve the hardware issue.

Can you tell me the clients for this controller and channels available?

In this case possibly we can dedicate one for NAND and keep the ones with
PIO mode dynamic in nature.. But yes it is not an elegant one.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

[ Dropping Mans to preserve his peace-of-mind ]

On 09/12/2016 18:56, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
>> On 09/12/2016 18:17, Vinod Koul wrote:
>>
>>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:

 What concrete solution do you propose?
>>>
>>> I have already proposed two solutions.
>>>
>>> A) Request a channel only when you need it. Obviously we can't do virtual
>>> channels with this (though we should still use virt-channels framework).
>>> The sbox setup and teardown can be done as part of channel request and
>>> freeup. PL08x already does this.
>>>
>>> Downside is that we can only have as many consumers at a time as channels.
>>>
>>> I have not heard any technical reason for not doing this apart from drivers
>>> grab the channel at probe, which is incorrect and needs to be fixed
>>> irrespective of the problem at hand.
>>>
>>> This is my preferred option.
>>
>> There is one important drawback with this solution. If a driver calls
>> dma_request_chan() when no channels are currently available, it will
>> get -EBUSY. If there were a flag in dma_request_chan to be put to
>> sleep (with timeout) until a channel is available, then it would
>> work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Maybe I'm missing something, but I don't see how that would help.
Take the NAND Flash controller driver, for instance. PIO is not
an option, because the ECC engine is tied to DMA.

And failing with -EAGAIN doesn't help the busy looping situation.
The caller should be put on some kind of queue to wait for a
"channel ready" event.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

[ Dropping Mans to preserve his peace-of-mind ]

On 09/12/2016 18:56, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
>> On 09/12/2016 18:17, Vinod Koul wrote:
>>
>>> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:

 What concrete solution do you propose?
>>>
>>> I have already proposed two solutions.
>>>
>>> A) Request a channel only when you need it. Obviously we can't do virtual
>>> channels with this (though we should still use virt-channels framework).
>>> The sbox setup and teardown can be done as part of channel request and
>>> freeup. PL08x already does this.
>>>
>>> Downside is that we can only have as many consumers at a time as channels.
>>>
>>> I have not heard any technical reason for not doing this apart from drivers
>>> grab the channel at probe, which is incorrect and needs to be fixed
>>> irrespective of the problem at hand.
>>>
>>> This is my preferred option.
>>
>> There is one important drawback with this solution. If a driver calls
>> dma_request_chan() when no channels are currently available, it will
>> get -EBUSY. If there were a flag in dma_request_chan to be put to
>> sleep (with timeout) until a channel is available, then it would
>> work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Maybe I'm missing something, but I don't see how that would help.
Take the NAND Flash controller driver, for instance. PIO is not
an option, because the ECC engine is tied to DMA.

And failing with -EAGAIN doesn't help the busy looping situation.
The caller should be put on some kind of queue to wait for a
"channel ready" event.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 11:26:06PM +0530, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> > On 09/12/2016 18:17, Vinod Koul wrote:
> > 
> > > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> > >>
> > >> What concrete solution do you propose?
> > > 
> > > I have already proposed two solutions.
> > > 
> > > A) Request a channel only when you need it. Obviously we can't do virtual
> > > channels with this (though we should still use virt-channels framework).
> > > The sbox setup and teardown can be done as part of channel request and
> > > freeup. PL08x already does this.
> > > 
> > > Downside is that we can only have as many consumers at a time as channels.
> > > 
> > > I have not heard any technical reason for not doing this apart from 
> > > drivers
> > > grab the channel at probe, which is incorrect and needs to be fixed
> > > irrespective of the problem at hand.
> > > 
> > > This is my preferred option.
> > 
> > There is one important drawback with this solution. If a driver calls
> > dma_request_chan() when no channels are currently available, it will
> > get -EBUSY. If there were a flag in dma_request_chan to be put to
> > sleep (with timeout) until a channel is available, then it would
> > work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Alternatively I can think of one more way.

If there is fixed delay or maximum delay predicted between ISR being
fired and transaction being completed from client, then we can use that
magic value and degrade the performance a bit but make a simpler system
than other two suggestions.

The idea here is that typically the subsequent transaction should be
issued as soon as possible, best case being in the ISR. But we can
degrade that performance a bit and issue in the tasklet. But that can be
done after introducing a delay, that too only in the case where new sbox
configuration is different from previous one (so performance degrade is
only on the switch and not for txn for same setup). You can possible
optimize even further by issuing in ISR for same sbox setup and issuing
in tasklet if configuration is different.

Yes this is bit iffy and adds more burden on driver, but lets us get
away with decent performance and being able to handle the hardware
condition.

Would that work for your case...?

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 11:26:06PM +0530, Vinod Koul wrote:
> On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> > On 09/12/2016 18:17, Vinod Koul wrote:
> > 
> > > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> > >>
> > >> What concrete solution do you propose?
> > > 
> > > I have already proposed two solutions.
> > > 
> > > A) Request a channel only when you need it. Obviously we can't do virtual
> > > channels with this (though we should still use virt-channels framework).
> > > The sbox setup and teardown can be done as part of channel request and
> > > freeup. PL08x already does this.
> > > 
> > > Downside is that we can only have as many consumers at a time as channels.
> > > 
> > > I have not heard any technical reason for not doing this apart from 
> > > drivers
> > > grab the channel at probe, which is incorrect and needs to be fixed
> > > irrespective of the problem at hand.
> > > 
> > > This is my preferred option.
> > 
> > There is one important drawback with this solution. If a driver calls
> > dma_request_chan() when no channels are currently available, it will
> > get -EBUSY. If there were a flag in dma_request_chan to be put to
> > sleep (with timeout) until a channel is available, then it would
> > work. But busy waiting in the client driver is a waste of power.
> 
> Right, but in that case the fallback would be PIO mode, and if that is
> not availble (IIRC some f your devices don't) then reject the usage with
> EAGAIN.

Alternatively I can think of one more way.

If there is fixed delay or maximum delay predicted between ISR being
fired and transaction being completed from client, then we can use that
magic value and degrade the performance a bit but make a simpler system
than other two suggestions.

The idea here is that typically the subsequent transaction should be
issued as soon as possible, best case being in the ISR. But we can
degrade that performance a bit and issue in the tasklet. But that can be
done after introducing a delay, that too only in the case where new sbox
configuration is different from previous one (so performance degrade is
only on the switch and not for txn for same setup). You can possible
optimize even further by issuing in ISR for same sbox setup and issuing
in tasklet if configuration is different.

Yes this is bit iffy and adds more burden on driver, but lets us get
away with decent performance and being able to handle the hardware
condition.

Would that work for your case...?

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> On 09/12/2016 18:17, Vinod Koul wrote:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>
> >> What concrete solution do you propose?
> > 
> > I have already proposed two solutions.
> > 
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> > 
> > Downside is that we can only have as many consumers at a time as channels.
> > 
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> > 
> > This is my preferred option.
> 
> There is one important drawback with this solution. If a driver calls
> dma_request_chan() when no channels are currently available, it will
> get -EBUSY. If there were a flag in dma_request_chan to be put to
> sleep (with timeout) until a channel is available, then it would
> work. But busy waiting in the client driver is a waste of power.

Right, but in that case the fallback would be PIO mode, and if that is
not availble (IIRC some f your devices don't) then reject the usage with
EAGAIN.


-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 06:34:15PM +0100, Mason wrote:
> On 09/12/2016 18:17, Vinod Koul wrote:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >>
> >> What concrete solution do you propose?
> > 
> > I have already proposed two solutions.
> > 
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> > 
> > Downside is that we can only have as many consumers at a time as channels.
> > 
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> > 
> > This is my preferred option.
> 
> There is one important drawback with this solution. If a driver calls
> dma_request_chan() when no channels are currently available, it will
> get -EBUSY. If there were a flag in dma_request_chan to be put to
> sleep (with timeout) until a channel is available, then it would
> work. But busy waiting in the client driver is a waste of power.

Right, but in that case the fallback would be PIO mode, and if that is
not availble (IIRC some f your devices don't) then reject the usage with
EAGAIN.


-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 05:28:01PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >> 
> >> What concrete solution do you propose?
> >
> > I have already proposed two solutions.
> >
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> >
> > Downside is that we can only have as many consumers at a time as channels.
> >
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> >
> > This is my preferred option.
> 
> Sorry, but this is not acceptable.

without outlining why..

> 
> > B) Create a custom driver specific API. This API for example:
> > sbox_setup(bool enable, ...)
> > can be called by client to explicitly setup and clear up the sbox setting.
> >
> > This way we can have transactions muxed.
> >
> > I have not heard any arguments on why we shouldn't do this except Russell's
> > comment that A) solves this.
> 
> Driver-specific interfaces are not the solution.  That way lies chaos
> and madness.

Yes fair enough. So would API change which 99% world doesnt need.

> This would all be so much easier if you all would just shut up for a
> moment and let me fix it properly.

Oh please go away, noone is asking you to reply!

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 05:28:01PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> >> 
> >> What concrete solution do you propose?
> >
> > I have already proposed two solutions.
> >
> > A) Request a channel only when you need it. Obviously we can't do virtual
> > channels with this (though we should still use virt-channels framework).
> > The sbox setup and teardown can be done as part of channel request and
> > freeup. PL08x already does this.
> >
> > Downside is that we can only have as many consumers at a time as channels.
> >
> > I have not heard any technical reason for not doing this apart from drivers
> > grab the channel at probe, which is incorrect and needs to be fixed
> > irrespective of the problem at hand.
> >
> > This is my preferred option.
> 
> Sorry, but this is not acceptable.

without outlining why..

> 
> > B) Create a custom driver specific API. This API for example:
> > sbox_setup(bool enable, ...)
> > can be called by client to explicitly setup and clear up the sbox setting.
> >
> > This way we can have transactions muxed.
> >
> > I have not heard any arguments on why we shouldn't do this except Russell's
> > comment that A) solves this.
> 
> Driver-specific interfaces are not the solution.  That way lies chaos
> and madness.

Yes fair enough. So would API change which 99% world doesnt need.

> This would all be so much easier if you all would just shut up for a
> moment and let me fix it properly.

Oh please go away, noone is asking you to reply!

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 09/12/2016 18:17, Vinod Koul wrote:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
> 
> I have already proposed two solutions.
> 
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
> 
> Downside is that we can only have as many consumers at a time as channels.
> 
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
> 
> This is my preferred option.

There is one important drawback with this solution. If a driver calls
dma_request_chan() when no channels are currently available, it will
get -EBUSY. If there were a flag in dma_request_chan to be put to
sleep (with timeout) until a channel is available, then it would
work. But busy waiting in the client driver is a waste of power.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 09/12/2016 18:17, Vinod Koul wrote:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>>
>> What concrete solution do you propose?
> 
> I have already proposed two solutions.
> 
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
> 
> Downside is that we can only have as many consumers at a time as channels.
> 
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
> 
> This is my preferred option.

There is one important drawback with this solution. If a driver calls
dma_request_chan() when no channels are currently available, it will
get -EBUSY. If there were a flag in dma_request_chan to be put to
sleep (with timeout) until a channel is available, then it would
work. But busy waiting in the client driver is a waste of power.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>> 
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.

Sorry, but this is not acceptable.

> B) Create a custom driver specific API. This API for example:
>   sbox_setup(bool enable, ...)
> can be called by client to explicitly setup and clear up the sbox setting.
>
> This way we can have transactions muxed.
>
> I have not heard any arguments on why we shouldn't do this except Russell's
> comment that A) solves this.

Driver-specific interfaces are not the solution.  That way lies chaos
and madness.

This would all be so much easier if you all would just shut up for a
moment and let me fix it properly.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
>> 
>> What concrete solution do you propose?
>
> I have already proposed two solutions.
>
> A) Request a channel only when you need it. Obviously we can't do virtual
> channels with this (though we should still use virt-channels framework).
> The sbox setup and teardown can be done as part of channel request and
> freeup. PL08x already does this.
>
> Downside is that we can only have as many consumers at a time as channels.
>
> I have not heard any technical reason for not doing this apart from drivers
> grab the channel at probe, which is incorrect and needs to be fixed
> irrespective of the problem at hand.
>
> This is my preferred option.

Sorry, but this is not acceptable.

> B) Create a custom driver specific API. This API for example:
>   sbox_setup(bool enable, ...)
> can be called by client to explicitly setup and clear up the sbox setting.
>
> This way we can have transactions muxed.
>
> I have not heard any arguments on why we shouldn't do this except Russell's
> comment that A) solves this.

Driver-specific interfaces are not the solution.  That way lies chaos
and madness.

This would all be so much easier if you all would just shut up for a
moment and let me fix it properly.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> 
> What concrete solution do you propose?

I have already proposed two solutions.

A) Request a channel only when you need it. Obviously we can't do virtual
channels with this (though we should still use virt-channels framework).
The sbox setup and teardown can be done as part of channel request and
freeup. PL08x already does this.

Downside is that we can only have as many consumers at a time as channels.

I have not heard any technical reason for not doing this apart from drivers
grab the channel at probe, which is incorrect and needs to be fixed
irrespective of the problem at hand.

This is my preferred option.

B) Create a custom driver specific API. This API for example:
sbox_setup(bool enable, ...)
can be called by client to explicitly setup and clear up the sbox setting.

This way we can have transactions muxed.

I have not heard any arguments on why we shouldn't do this except Russell's
comment that A) solves this.

> Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
> arrives "too soon") in a different way.
> Instead of thinking the IRQ indicates "transfer complete", it is indicating 
> "ready
> to accept another command", which in practice (and with proper API support) 
> can
> translate into efficient queuing of DMA operations.

That IMO is a better understanding of this issue. But based on discussion, I
think the issue is that submitting next transaction cannot be done until
sbox is setup (as a consequence torn down for previous). Tear down can be
down only when client knows that transfer is done, from dma controller data
has been pushed and in-flight.


-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Fri, Dec 09, 2016 at 11:25:57AM +0100, Sebastian Frias wrote:
> 
> What concrete solution do you propose?

I have already proposed two solutions.

A) Request a channel only when you need it. Obviously we can't do virtual
channels with this (though we should still use virt-channels framework).
The sbox setup and teardown can be done as part of channel request and
freeup. PL08x already does this.

Downside is that we can only have as many consumers at a time as channels.

I have not heard any technical reason for not doing this apart from drivers
grab the channel at probe, which is incorrect and needs to be fixed
irrespective of the problem at hand.

This is my preferred option.

B) Create a custom driver specific API. This API for example:
sbox_setup(bool enable, ...)
can be called by client to explicitly setup and clear up the sbox setting.

This way we can have transactions muxed.

I have not heard any arguments on why we shouldn't do this except Russell's
comment that A) solves this.

> Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
> arrives "too soon") in a different way.
> Instead of thinking the IRQ indicates "transfer complete", it is indicating 
> "ready
> to accept another command", which in practice (and with proper API support) 
> can
> translate into efficient queuing of DMA operations.

That IMO is a better understanding of this issue. But based on discussion, I
think the issue is that submitting next transaction cannot be done until
sbox is setup (as a consequence torn down for previous). Tear down can be
down only when client knows that transfer is done, from dma controller data
has been pushed and in-flight.


-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[1Måns Rullgård]

Sebastian Frias  writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>>
 To make it efficient, disregarding your Sbox HW issue, the solution is
 virtual channels. You can delink physical channels and virtual channels. If
 one has SW controlled MUX then a channel can service any client. For few
 controllers request lines are hard wired so they cant use any channel. But
 if you dont have this restriction then driver can queue up many 
 transactions
 from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush at the end of a series of transfers, before switching the
channel to another device.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[1Måns Rullgård]

Sebastian Frias  writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>>
 To make it efficient, disregarding your Sbox HW issue, the solution is
 virtual channels. You can delink physical channels and virtual channels. If
 one has SW controlled MUX then a channel can service any client. For few
 controllers request lines are hard wired so they cant use any channel. But
 if you dont have this restriction then driver can queue up many 
 transactions
 from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush at the end of a series of transfers, before switching the
channel to another device.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Sebastian Frias  writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>>
 To make it efficient, disregarding your Sbox HW issue, the solution is
 virtual channels. You can delink physical channels and virtual channels. If
 one has SW controlled MUX then a channel can service any client. For few
 controllers request lines are hard wired so they cant use any channel. But
 if you dont have this restriction then driver can queue up many 
 transactions
 from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush 

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Sebastian Frias  writes:

> On 09/12/16 07:59, Vinod Koul wrote:
>> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>>
 To make it efficient, disregarding your Sbox HW issue, the solution is
 virtual channels. You can delink physical channels and virtual channels. If
 one has SW controlled MUX then a channel can service any client. For few
 controllers request lines are hard wired so they cant use any channel. But
 if you dont have this restriction then driver can queue up many 
 transactions
 from different controllers.
>>>
>>> Have you been paying attention at all?  This exactly what the driver
>>> ALREADY DOES.
>> 
>> And have you read what the question was?

I wrote the driver.  I think I know what Mason and I are asking.

> I think many people appreciate the quick turn around time and
> responsiveness of knowledgeable people making constructive remarks in
> this thread, but it looks we are slowly drifting away from the main
> problem.
>
> If we had to sum up the discussion, the current DMA API/framework in
> Linux seems to lack a way to properly handle this HW (or if it has a
> way, the information got lost somewhere).
>
> What concrete solution do you propose?
>
> Alternatively, one can think of the current issue (i.e.: the fact that
> the IRQ arrives "too soon") in a different way.  Instead of thinking
> the IRQ indicates "transfer complete", it is indicating "ready to
> accept another command", which in practice (and with proper API
> support) can translate into efficient queuing of DMA operations.

For multiple back to back transfers to the same peripheral, it is indeed
a slight optimisation.  What's apparently lacking is some way of doing a
full flush 

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Sebastian Frias]

On 09/12/16 07:59, Vinod Koul wrote:
> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>>
>>> To make it efficient, disregarding your Sbox HW issue, the solution is
>>> virtual channels. You can delink physical channels and virtual channels. If
>>> one has SW controlled MUX then a channel can service any client. For few
>>> controllers request lines are hard wired so they cant use any channel. But
>>> if you dont have this restriction then driver can queue up many transactions
>>> from different controllers.
>>
>> Have you been paying attention at all?  This exactly what the driver
>> ALREADY DOES.
> 
> And have you read what the question was?
> 

I think many people appreciate the quick turn around time and responsiveness of
knowledgeable people making constructive remarks in this thread, but it looks we
are slowly drifting away from the main problem.

If we had to sum up the discussion, the current DMA API/framework in Linux seems
to lack a way to properly handle this HW (or if it has a way, the information 
got
lost somewhere).

What concrete solution do you propose?

Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
arrives "too soon") in a different way.
Instead of thinking the IRQ indicates "transfer complete", it is indicating 
"ready
to accept another command", which in practice (and with proper API support) can
translate into efficient queuing of DMA operations.

Re: Tearing down DMA transfer setup after DMA client has finished

[Sebastian Frias]

On 09/12/16 07:59, Vinod Koul wrote:
> On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>>
>>> To make it efficient, disregarding your Sbox HW issue, the solution is
>>> virtual channels. You can delink physical channels and virtual channels. If
>>> one has SW controlled MUX then a channel can service any client. For few
>>> controllers request lines are hard wired so they cant use any channel. But
>>> if you dont have this restriction then driver can queue up many transactions
>>> from different controllers.
>>
>> Have you been paying attention at all?  This exactly what the driver
>> ALREADY DOES.
> 
> And have you read what the question was?
> 

I think many people appreciate the quick turn around time and responsiveness of
knowledgeable people making constructive remarks in this thread, but it looks we
are slowly drifting away from the main problem.

If we had to sum up the discussion, the current DMA API/framework in Linux seems
to lack a way to properly handle this HW (or if it has a way, the information 
got
lost somewhere).

What concrete solution do you propose?

Alternatively, one can think of the current issue (i.e.: the fact that the IRQ
arrives "too soon") in a different way.
Instead of thinking the IRQ indicates "transfer complete", it is indicating 
"ready
to accept another command", which in practice (and with proper API support) can
translate into efficient queuing of DMA operations.

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > To make it efficient, disregarding your Sbox HW issue, the solution is
> > virtual channels. You can delink physical channels and virtual channels. If
> > one has SW controlled MUX then a channel can service any client. For few
> > controllers request lines are hard wired so they cant use any channel. But
> > if you dont have this restriction then driver can queue up many transactions
> > from different controllers.
> 
> Have you been paying attention at all?  This exactly what the driver
> ALREADY DOES.

And have you read what the question was?

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 04:48:18PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > To make it efficient, disregarding your Sbox HW issue, the solution is
> > virtual channels. You can delink physical channels and virtual channels. If
> > one has SW controlled MUX then a channel can service any client. For few
> > controllers request lines are hard wired so they cant use any channel. But
> > if you dont have this restriction then driver can queue up many transactions
> > from different controllers.
> 
> Have you been paying attention at all?  This exactly what the driver
> ALREADY DOES.

And have you read what the question was?

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> To make it efficient, disregarding your Sbox HW issue, the solution is
> virtual channels. You can delink physical channels and virtual channels. If
> one has SW controlled MUX then a channel can service any client. For few
> controllers request lines are hard wired so they cant use any channel. But
> if you dont have this restriction then driver can queue up many transactions
> from different controllers.

Have you been paying attention at all?  This exactly what the driver
ALREADY DOES.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> To make it efficient, disregarding your Sbox HW issue, the solution is
> virtual channels. You can delink physical channels and virtual channels. If
> one has SW controlled MUX then a channel can service any client. For few
> controllers request lines are hard wired so they cant use any channel. But
> if you dont have this restriction then driver can queue up many transactions
> from different controllers.

Have you been paying attention at all?  This exactly what the driver
ALREADY DOES.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Thu, Dec 08, 2016 at 11:44:53AM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> 
>> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> >> Vinod Koul  writes:
>> >> 
>> >> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> >> >> 
>> >> >> That's not going to work very well.  Device drivers typically request
>> >> >> dma channels in their probe functions or when the device is opened.
>> >> >> This means that reserving one of the few channels there will inevitably
>> >> >> make some other device fail to operate.
>> >> >
>> >> > No that doesnt make sense at all, you should get a channel only when you
>> >> > want to use it and not in probe!
>> >> 
>> >> Tell that to just about every single driver ever written.
>> >
>> > Not really, few do yes which is wrong but not _all_ do that.
>> 
>> Every driver I ever looked at does.  Name one you consider "correct."
>
> That only tells me you haven't looked enough and want to rant!
>
> FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> could find other examples and go on and on, but that's besides the point and
> looks like you don't want to listen to people telling you something..

ALSA is a particularly poor example.  ALSA drivers request a dma channel
at some point between the device being opened and playback (or capture)
starting.  They then set up a cyclic transfer that runs continuously
until playback is stopped.  It's a completely different model of
operation than we're talking about here.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Thu, Dec 08, 2016 at 11:44:53AM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> 
>> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> >> Vinod Koul  writes:
>> >> 
>> >> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> >> >> 
>> >> >> That's not going to work very well.  Device drivers typically request
>> >> >> dma channels in their probe functions or when the device is opened.
>> >> >> This means that reserving one of the few channels there will inevitably
>> >> >> make some other device fail to operate.
>> >> >
>> >> > No that doesnt make sense at all, you should get a channel only when you
>> >> > want to use it and not in probe!
>> >> 
>> >> Tell that to just about every single driver ever written.
>> >
>> > Not really, few do yes which is wrong but not _all_ do that.
>> 
>> Every driver I ever looked at does.  Name one you consider "correct."
>
> That only tells me you haven't looked enough and want to rant!
>
> FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> could find other examples and go on and on, but that's besides the point and
> looks like you don't want to listen to people telling you something..

ALSA is a particularly poor example.  ALSA drivers request a dma channel
at some point between the device being opened and playback (or capture)
starting.  They then set up a cyclic transfer that runs continuously
until playback is stopped.  It's a completely different model of
operation than we're talking about here.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 11:54:51AM +0100, Mason wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> >
> >> Vinod Koul  writes:
> >>
> >>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> 
>  That's not going to work very well.  Device drivers typically request
>  dma channels in their probe functions or when the device is opened.
>  This means that reserving one of the few channels there will inevitably
>  make some other device fail to operate.
> >>>
> >>> No that doesn't make sense at all, you should get a channel only when you
> >>> want to use it and not in probe!
> >>
> >> Tell that to just about every single driver ever written.
> > 
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Vinod,
> 
> Could you explain something to me in layman's terms?
> 
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
> 
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
> 
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
> 
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.

That is correct

> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

If someone else is using the channels then the bust wait will not help, so
in this case you should fall back to PIO, few drivers do that

> I don't understand how the multiplexing of few memory
> channels to many clients is supposed to happen efficiently?

To make it efficient, disregarding your Sbox HW issue, the solution is
virtual channels. You can delink physical channels and virtual channels. If
one has SW controlled MUX then a channel can service any client. For few
controllers request lines are hard wired so they cant use any channel. But
if you dont have this restriction then driver can queue up many transactions
from different controllers.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 11:54:51AM +0100, Mason wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> >
> >> Vinod Koul  writes:
> >>
> >>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> 
>  That's not going to work very well.  Device drivers typically request
>  dma channels in their probe functions or when the device is opened.
>  This means that reserving one of the few channels there will inevitably
>  make some other device fail to operate.
> >>>
> >>> No that doesn't make sense at all, you should get a channel only when you
> >>> want to use it and not in probe!
> >>
> >> Tell that to just about every single driver ever written.
> > 
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Vinod,
> 
> Could you explain something to me in layman's terms?
> 
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
> 
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
> 
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
> 
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.

That is correct

> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

If someone else is using the channels then the bust wait will not help, so
in this case you should fall back to PIO, few drivers do that

> I don't understand how the multiplexing of few memory
> channels to many clients is supposed to happen efficiently?

To make it efficient, disregarding your Sbox HW issue, the solution is
virtual channels. You can delink physical channels and virtual channels. If
one has SW controlled MUX then a channel can service any client. For few
controllers request lines are hard wired so they cant use any channel. But
if you dont have this restriction then driver can queue up many transactions
from different controllers.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Thu, Dec 08, 2016 at 12:20:30PM +, Måns Rullgård wrote:
>> >
>> > I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
>> > it does request DMA channels at open time, not at probe time.
>> 
>> In the part quoted above, I said most drivers request dma channels in
>> their probe or open functions.  For the purposes of this discussion,
>> that distinction is irrelevant.  In either case, the channel is held
>> indefinitely.  
>
> And the answer was it is wrong and not _all_ do that!!

Show me one that doesn't.

>> If this wasn't the correct way to use the dmaengine,
>> there would be no need for the virt-dma helpers which are specifically
>> designed for cases the one currently at hand.
>
> That is incorrect.
>
> virt-dma helps to have multiple request from various clients. For many
> controllers which implement a SW mux, they can transfer data for one client
> and then next transfer can be for some other one.
>
> This allows better utilization of dma channels and helps in case where we
> have fewer dma channels than users.

Which is *exactly* the situation we have here.

I have no idea what you're arguing for or against any more.  Perhaps
you're just arguing.

>> The only problem we have is that nobody envisioned hardware where the
>> dma engine indicates completion slightly too soon.  I suspect there's a
>> fifo or such somewhere, and the interrupt is triggered when the last
>> byte has been placed in the fifo rather than when it has been removed
>> which would have been more correct.
>
> That is pretty common hardware optimization but usually hardware shows up
> with flush commands to let in flight transactions be completed.

Well, this hardware seems to lack that particular feature.  Pretending
otherwise isn't helping.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Thu, Dec 08, 2016 at 12:20:30PM +, Måns Rullgård wrote:
>> >
>> > I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
>> > it does request DMA channels at open time, not at probe time.
>> 
>> In the part quoted above, I said most drivers request dma channels in
>> their probe or open functions.  For the purposes of this discussion,
>> that distinction is irrelevant.  In either case, the channel is held
>> indefinitely.  
>
> And the answer was it is wrong and not _all_ do that!!

Show me one that doesn't.

>> If this wasn't the correct way to use the dmaengine,
>> there would be no need for the virt-dma helpers which are specifically
>> designed for cases the one currently at hand.
>
> That is incorrect.
>
> virt-dma helps to have multiple request from various clients. For many
> controllers which implement a SW mux, they can transfer data for one client
> and then next transfer can be for some other one.
>
> This allows better utilization of dma channels and helps in case where we
> have fewer dma channels than users.

Which is *exactly* the situation we have here.

I have no idea what you're arguing for or against any more.  Perhaps
you're just arguing.

>> The only problem we have is that nobody envisioned hardware where the
>> dma engine indicates completion slightly too soon.  I suspect there's a
>> fifo or such somewhere, and the interrupt is triggered when the last
>> byte has been placed in the fifo rather than when it has been removed
>> which would have been more correct.
>
> That is pretty common hardware optimization but usually hardware shows up
> with flush commands to let in flight transactions be completed.

Well, this hardware seems to lack that particular feature.  Pretending
otherwise isn't helping.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 04:43:53PM +0100, Mason wrote:
> On 08/12/2016 16:40, Vinod Koul wrote:
> 
> > FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> > could find other examples and go on and on, but that's besides the point and
> > looks like you don't want to listen to people telling you something..
> 
> Hello Vinod,
> 
> FWIW, your system clock seems to be 10 minutes ahead ;-)

Ah thanks for pointing, fixed now :)
> 
> I am willing to learn. I have a few outstanding questions
> in the thread. Could you have a look at them?

Sure thing, I have replied/ing to few things, feel free to ping me if I miss
something today..

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 04:43:53PM +0100, Mason wrote:
> On 08/12/2016 16:40, Vinod Koul wrote:
> 
> > FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> > could find other examples and go on and on, but that's besides the point and
> > looks like you don't want to listen to people telling you something..
> 
> Hello Vinod,
> 
> FWIW, your system clock seems to be 10 minutes ahead ;-)

Ah thanks for pointing, fixed now :)
> 
> I am willing to learn. I have a few outstanding questions
> in the thread. Could you have a look at them?

Sure thing, I have replied/ing to few things, feel free to ping me if I miss
something today..

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 16:40, Vinod Koul wrote:

> FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> could find other examples and go on and on, but that's besides the point and
> looks like you don't want to listen to people telling you something..

Hello Vinod,

FWIW, your system clock seems to be 10 minutes ahead ;-)

I am willing to learn. I have a few outstanding questions
in the thread. Could you have a look at them?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 16:40, Vinod Koul wrote:

> FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
> could find other examples and go on and on, but that's besides the point and
> looks like you don't want to listen to people telling you something..

Hello Vinod,

FWIW, your system clock seems to be 10 minutes ahead ;-)

I am willing to learn. I have a few outstanding questions
in the thread. Could you have a look at them?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 12:20:30PM +, Måns Rullgård wrote:
> >
> > I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> > it does request DMA channels at open time, not at probe time.
> 
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  

And the answer was it is wrong and not _all_ do that!!

> If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

That is incorrect.

virt-dma helps to have multiple request from various clients. For many
controllers which implement a SW mux, they can transfer data for one client
and then next transfer can be for some other one.

This allows better utilization of dma channels and helps in case where we
have fewer dma channels than users. This was _not_ developed to let people
grab channels in probe.

> The only problem we have is that nobody envisioned hardware where the
> dma engine indicates completion slightly too soon.  I suspect there's a
> fifo or such somewhere, and the interrupt is triggered when the last
> byte has been placed in the fifo rather than when it has been removed
> which would have been more correct.

That is pretty common hardware optimization but usually hardware shows up
with flush commands to let in flight transactions be completed.

One other example of this implementations has already been pointed in this
thread

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 12:20:30PM +, Måns Rullgård wrote:
> >
> > I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> > it does request DMA channels at open time, not at probe time.
> 
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  

And the answer was it is wrong and not _all_ do that!!

> If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

That is incorrect.

virt-dma helps to have multiple request from various clients. For many
controllers which implement a SW mux, they can transfer data for one client
and then next transfer can be for some other one.

This allows better utilization of dma channels and helps in case where we
have fewer dma channels than users. This was _not_ developed to let people
grab channels in probe.

> The only problem we have is that nobody envisioned hardware where the
> dma engine indicates completion slightly too soon.  I suspect there's a
> fifo or such somewhere, and the interrupt is triggered when the last
> byte has been placed in the fifo rather than when it has been removed
> which would have been more correct.

That is pretty common hardware optimization but usually hardware shows up
with flush commands to let in flight transactions be completed.

One other example of this implementations has already been pointed in this
thread

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 11:44:53AM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> >> Vinod Koul  writes:
> >> 
> >> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> >> 
> >> >> That's not going to work very well.  Device drivers typically request
> >> >> dma channels in their probe functions or when the device is opened.
> >> >> This means that reserving one of the few channels there will inevitably
> >> >> make some other device fail to operate.
> >> >
> >> > No that doesnt make sense at all, you should get a channel only when you
> >> > want to use it and not in probe!
> >> 
> >> Tell that to just about every single driver ever written.
> >
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Every driver I ever looked at does.  Name one you consider "correct."

That only tells me you haven't looked enough and want to rant!

FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
could find other examples and go on and on, but that's besides the point and
looks like you don't want to listen to people telling you something..

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Thu, Dec 08, 2016 at 11:44:53AM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> >> Vinod Koul  writes:
> >> 
> >> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> >> 
> >> >> That's not going to work very well.  Device drivers typically request
> >> >> dma channels in their probe functions or when the device is opened.
> >> >> This means that reserving one of the few channels there will inevitably
> >> >> make some other device fail to operate.
> >> >
> >> > No that doesnt make sense at all, you should get a channel only when you
> >> > want to use it and not in probe!
> >> 
> >> Tell that to just about every single driver ever written.
> >
> > Not really, few do yes which is wrong but not _all_ do that.
> 
> Every driver I ever looked at does.  Name one you consider "correct."

That only tells me you haven't looked enough and want to rant!

FWIW look at ALSA-dmaengine lib, thereby every audio driver that uses it. I
could find other examples and go on and on, but that's besides the point and
looks like you don't want to listen to people telling you something..

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 08/12/2016 13:44, Måns Rullgård wrote:
>
>> Mason  writes:
>> 
>>> On 08/12/2016 13:20, Måns Rullgård wrote:
>>>
 The only problem we have is that nobody envisioned hardware where the
 dma engine indicates completion slightly too soon.  I suspect there's a
 fifo or such somewhere, and the interrupt is triggered when the last
 byte has been placed in the fifo rather than when it has been removed
 which would have been more correct.
>>>
>>> As I (tried to) explain here:
>>> https://marc.info/?l=dmaengine=148007808418242=2
>>>
>>> A *read* MBUS agent raises its IRQ when it is safe for the memory
>>> to be overwritten (i.e. every byte has been pushed into the pipe).
>>>
>>> A *write* MBUS agent raises its IRQ when it is safe for another
>>> agent to read any one of the transferred bytes.
>>>
>>> The issue comes from the fact that, for a memory-to-device transfer,
>>> the system will receive the read agent's IRQ, but most devices
>>> (NFC, SATA) don't have an IRQ line to signal that their part of the
>>> operation is complete.
>> 
>> SATA does, actually.  Nevertheless, it's an unusual design.
>
> Thanks, I was mistaken about the SATA controller.
>
> On tango3 (and also tango4, I assume)
>
> IRQ 41 = Serial ATA #0
> IRQ 42 = Serial ATA DMA #0
> IRQ 54 = Serial ATA #1
> IRQ 55 = Serial ATA DMA #1
>
> But in the end, whether there is a device interrupt (SATA)
> or not (NFC), for a memory-to-device transfer, the DMA
> driver will get the read agent notification (which should
> be ignored) and the client driver should either spin until
> idle (NFC) or wait for its completion IRQ (SATA).
>
> Correct?

Yes, and when the client device is finished, the driver needs to signal
this to the dma driver so it can reuse the channel.  It's this last
piece that's missing.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 08/12/2016 13:44, Måns Rullgård wrote:
>
>> Mason  writes:
>> 
>>> On 08/12/2016 13:20, Måns Rullgård wrote:
>>>
 The only problem we have is that nobody envisioned hardware where the
 dma engine indicates completion slightly too soon.  I suspect there's a
 fifo or such somewhere, and the interrupt is triggered when the last
 byte has been placed in the fifo rather than when it has been removed
 which would have been more correct.
>>>
>>> As I (tried to) explain here:
>>> https://marc.info/?l=dmaengine=148007808418242=2
>>>
>>> A *read* MBUS agent raises its IRQ when it is safe for the memory
>>> to be overwritten (i.e. every byte has been pushed into the pipe).
>>>
>>> A *write* MBUS agent raises its IRQ when it is safe for another
>>> agent to read any one of the transferred bytes.
>>>
>>> The issue comes from the fact that, for a memory-to-device transfer,
>>> the system will receive the read agent's IRQ, but most devices
>>> (NFC, SATA) don't have an IRQ line to signal that their part of the
>>> operation is complete.
>> 
>> SATA does, actually.  Nevertheless, it's an unusual design.
>
> Thanks, I was mistaken about the SATA controller.
>
> On tango3 (and also tango4, I assume)
>
> IRQ 41 = Serial ATA #0
> IRQ 42 = Serial ATA DMA #0
> IRQ 54 = Serial ATA #1
> IRQ 55 = Serial ATA DMA #1
>
> But in the end, whether there is a device interrupt (SATA)
> or not (NFC), for a memory-to-device transfer, the DMA
> driver will get the read agent notification (which should
> be ignored) and the client driver should either spin until
> idle (NFC) or wait for its completion IRQ (SATA).
>
> Correct?

Yes, and when the client device is finished, the driver needs to signal
this to the dma driver so it can reuse the channel.  It's this last
piece that's missing.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:44, Måns Rullgård wrote:

> Mason  writes:
> 
>> On 08/12/2016 13:20, Måns Rullgård wrote:
>>
>>> The only problem we have is that nobody envisioned hardware where the
>>> dma engine indicates completion slightly too soon.  I suspect there's a
>>> fifo or such somewhere, and the interrupt is triggered when the last
>>> byte has been placed in the fifo rather than when it has been removed
>>> which would have been more correct.
>>
>> As I (tried to) explain here:
>> https://marc.info/?l=dmaengine=148007808418242=2
>>
>> A *read* MBUS agent raises its IRQ when it is safe for the memory
>> to be overwritten (i.e. every byte has been pushed into the pipe).
>>
>> A *write* MBUS agent raises its IRQ when it is safe for another
>> agent to read any one of the transferred bytes.
>>
>> The issue comes from the fact that, for a memory-to-device transfer,
>> the system will receive the read agent's IRQ, but most devices
>> (NFC, SATA) don't have an IRQ line to signal that their part of the
>> operation is complete.
> 
> SATA does, actually.  Nevertheless, it's an unusual design.

Thanks, I was mistaken about the SATA controller.

On tango3 (and also tango4, I assume)

IRQ 41 = Serial ATA #0
IRQ 42 = Serial ATA DMA #0
IRQ 54 = Serial ATA #1
IRQ 55 = Serial ATA DMA #1

But in the end, whether there is a device interrupt (SATA)
or not (NFC), for a memory-to-device transfer, the DMA
driver will get the read agent notification (which should
be ignored) and the client driver should either spin until
idle (NFC) or wait for its completion IRQ (SATA).

Correct?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:44, Måns Rullgård wrote:

> Mason  writes:
> 
>> On 08/12/2016 13:20, Måns Rullgård wrote:
>>
>>> The only problem we have is that nobody envisioned hardware where the
>>> dma engine indicates completion slightly too soon.  I suspect there's a
>>> fifo or such somewhere, and the interrupt is triggered when the last
>>> byte has been placed in the fifo rather than when it has been removed
>>> which would have been more correct.
>>
>> As I (tried to) explain here:
>> https://marc.info/?l=dmaengine=148007808418242=2
>>
>> A *read* MBUS agent raises its IRQ when it is safe for the memory
>> to be overwritten (i.e. every byte has been pushed into the pipe).
>>
>> A *write* MBUS agent raises its IRQ when it is safe for another
>> agent to read any one of the transferred bytes.
>>
>> The issue comes from the fact that, for a memory-to-device transfer,
>> the system will receive the read agent's IRQ, but most devices
>> (NFC, SATA) don't have an IRQ line to signal that their part of the
>> operation is complete.
> 
> SATA does, actually.  Nevertheless, it's an unusual design.

Thanks, I was mistaken about the SATA controller.

On tango3 (and also tango4, I assume)

IRQ 41 = Serial ATA #0
IRQ 42 = Serial ATA DMA #0
IRQ 54 = Serial ATA #1
IRQ 55 = Serial ATA DMA #1

But in the end, whether there is a device interrupt (SATA)
or not (NFC), for a memory-to-device transfer, the DMA
driver will get the read agent notification (which should
be ignored) and the client driver should either spin until
idle (NFC) or wait for its completion IRQ (SATA).

Correct?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 08/12/2016 13:20, Måns Rullgård wrote:
>
>> The only problem we have is that nobody envisioned hardware where the
>> dma engine indicates completion slightly too soon.  I suspect there's a
>> fifo or such somewhere, and the interrupt is triggered when the last
>> byte has been placed in the fifo rather than when it has been removed
>> which would have been more correct.
>
> As I (tried to) explain here:
> https://marc.info/?l=dmaengine=148007808418242=2
>
> A *read* MBUS agent raises its IRQ when it is safe for the memory
> to be overwritten (i.e. every byte has been pushed into the pipe).
>
> A *write* MBUS agent raises its IRQ when it is safe for another
> agent to read any one of the transferred bytes.
>
> The issue comes from the fact that, for a memory-to-device transfer,
> the system will receive the read agent's IRQ, but most devices
> (NFC, SATA) don't have an IRQ line to signal that their part of the
> operation is complete.

SATA does, actually.  Nevertheless, it's an unusual design.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 08/12/2016 13:20, Måns Rullgård wrote:
>
>> The only problem we have is that nobody envisioned hardware where the
>> dma engine indicates completion slightly too soon.  I suspect there's a
>> fifo or such somewhere, and the interrupt is triggered when the last
>> byte has been placed in the fifo rather than when it has been removed
>> which would have been more correct.
>
> As I (tried to) explain here:
> https://marc.info/?l=dmaengine=148007808418242=2
>
> A *read* MBUS agent raises its IRQ when it is safe for the memory
> to be overwritten (i.e. every byte has been pushed into the pipe).
>
> A *write* MBUS agent raises its IRQ when it is safe for another
> agent to read any one of the transferred bytes.
>
> The issue comes from the fact that, for a memory-to-device transfer,
> the system will receive the read agent's IRQ, but most devices
> (NFC, SATA) don't have an IRQ line to signal that their part of the
> operation is complete.

SATA does, actually.  Nevertheless, it's an unusual design.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:20, Måns Rullgård wrote:

> The only problem we have is that nobody envisioned hardware where the
> dma engine indicates completion slightly too soon.  I suspect there's a
> fifo or such somewhere, and the interrupt is triggered when the last
> byte has been placed in the fifo rather than when it has been removed
> which would have been more correct.

As I (tried to) explain here:
https://marc.info/?l=dmaengine=148007808418242=2

A *read* MBUS agent raises its IRQ when it is safe for the memory
to be overwritten (i.e. every byte has been pushed into the pipe).

A *write* MBUS agent raises its IRQ when it is safe for another
agent to read any one of the transferred bytes.

The issue comes from the fact that, for a memory-to-device transfer,
the system will receive the read agent's IRQ, but most devices
(NFC, SATA) don't have an IRQ line to signal that their part of the
operation is complete.

As I explained, if one sets up a memory-to-memory DMA copy, then
the system will actually receive *two* IRQs.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:20, Måns Rullgård wrote:

> The only problem we have is that nobody envisioned hardware where the
> dma engine indicates completion slightly too soon.  I suspect there's a
> fifo or such somewhere, and the interrupt is triggered when the last
> byte has been placed in the fifo rather than when it has been removed
> which would have been more correct.

As I (tried to) explain here:
https://marc.info/?l=dmaengine=148007808418242=2

A *read* MBUS agent raises its IRQ when it is safe for the memory
to be overwritten (i.e. every byte has been pushed into the pipe).

A *write* MBUS agent raises its IRQ when it is safe for another
agent to read any one of the transferred bytes.

The issue comes from the fact that, for a memory-to-device transfer,
the system will receive the read agent's IRQ, but most devices
(NFC, SATA) don't have an IRQ line to signal that their part of the
operation is complete.

As I explained, if one sets up a memory-to-memory DMA copy, then
the system will actually receive *two* IRQs.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 1:20 PM, Måns Rullgård  wrote:
> Geert Uytterhoeven  writes:
>> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
>>> Vinod Koul  writes:
 On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> That's not going to work very well.  Device drivers typically request
> >> dma channels in their probe functions or when the device is opened.
> >> This means that reserving one of the few channels there will inevitably
> >> make some other device fail to operate.
> >
> > No that doesnt make sense at all, you should get a channel only when you
> > want to use it and not in probe!
>
> Tell that to just about every single driver ever written.

 Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Every driver I ever looked at does.  Name one you consider "correct."
>>
>> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
>> it does request DMA channels at open time, not at probe time.
>
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

Sorry, I mainly read Vinod's "not in probe", and missed your "or when the
device is opened".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 1:20 PM, Måns Rullgård  wrote:
> Geert Uytterhoeven  writes:
>> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
>>> Vinod Koul  writes:
 On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> That's not going to work very well.  Device drivers typically request
> >> dma channels in their probe functions or when the device is opened.
> >> This means that reserving one of the few channels there will inevitably
> >> make some other device fail to operate.
> >
> > No that doesnt make sense at all, you should get a channel only when you
> > want to use it and not in probe!
>
> Tell that to just about every single driver ever written.

 Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Every driver I ever looked at does.  Name one you consider "correct."
>>
>> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
>> it does request DMA channels at open time, not at probe time.
>
> In the part quoted above, I said most drivers request dma channels in
> their probe or open functions.  For the purposes of this discussion,
> that distinction is irrelevant.  In either case, the channel is held
> indefinitely.  If this wasn't the correct way to use the dmaengine,
> there would be no need for the virt-dma helpers which are specifically
> designed for cases the one currently at hand.

Sorry, I mainly read Vinod's "not in probe", and missed your "or when the
device is opened".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> Hi Måns,
>
> On Thu, Dec 8, 2016 at 12:47 PM, Måns Rullgård  wrote:
>> Geert Uytterhoeven  writes:
>>> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
 On 08/12/2016 11:39, Vinod Koul wrote:
> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
 That's not going to work very well.  Device drivers typically request
 dma channels in their probe functions or when the device is opened.
 This means that reserving one of the few channels there will inevitably
 make some other device fail to operate.
>>>
>>> No that doesn't make sense at all, you should get a channel only when 
>>> you
>>> want to use it and not in probe!
>>
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

 Vinod,

 Could you explain something to me in layman's terms?

 I have a NAND Flash Controller driver that depends on the
 DMA driver under discussion.

 Suppose I move the dma_request_chan() call from the driver's
 probe function, to the actual DMA transfer function.

 I would want dma_request_chan() to put the calling thread
 to sleep until a channel becomes available (possibly with
 a timeout value).

 But Maxime told me dma_request_chan() will just return
 -EBUSY if no channels are available.

 Am I supposed to busy wait in my driver's DMA function
 until a channel becomes available?
>>>
>>> Can you fall back to PIO if requesting a channel fails?
>>>
>>> Alternatively, dma_request_chan() could always succeed, and
>>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>>> available due to a limitation on the number of active channels, and
>>> the driver could fall back to PIO for that transfer.
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
>
> You still want to fall back to PIO if the DMA engine is not available at all
> (e.g. DMA engine driver not compiled in, or module not loaded).

That's a choice for each device driver to make.  Some devices don't have
a pio mode at all.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> Hi Måns,
>
> On Thu, Dec 8, 2016 at 12:47 PM, Måns Rullgård  wrote:
>> Geert Uytterhoeven  writes:
>>> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
 On 08/12/2016 11:39, Vinod Koul wrote:
> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
 That's not going to work very well.  Device drivers typically request
 dma channels in their probe functions or when the device is opened.
 This means that reserving one of the few channels there will inevitably
 make some other device fail to operate.
>>>
>>> No that doesn't make sense at all, you should get a channel only when 
>>> you
>>> want to use it and not in probe!
>>
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

 Vinod,

 Could you explain something to me in layman's terms?

 I have a NAND Flash Controller driver that depends on the
 DMA driver under discussion.

 Suppose I move the dma_request_chan() call from the driver's
 probe function, to the actual DMA transfer function.

 I would want dma_request_chan() to put the calling thread
 to sleep until a channel becomes available (possibly with
 a timeout value).

 But Maxime told me dma_request_chan() will just return
 -EBUSY if no channels are available.

 Am I supposed to busy wait in my driver's DMA function
 until a channel becomes available?
>>>
>>> Can you fall back to PIO if requesting a channel fails?
>>>
>>> Alternatively, dma_request_chan() could always succeed, and
>>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>>> available due to a limitation on the number of active channels, and
>>> the driver could fall back to PIO for that transfer.
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
>
> You still want to fall back to PIO if the DMA engine is not available at all
> (e.g. DMA engine driver not compiled in, or module not loaded).

That's a choice for each device driver to make.  Some devices don't have
a pio mode at all.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
>> Vinod Koul  writes:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
 Vinod Koul  writes:
 > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
 >> That's not going to work very well.  Device drivers typically request
 >> dma channels in their probe functions or when the device is opened.
 >> This means that reserving one of the few channels there will inevitably
 >> make some other device fail to operate.
 >
 > No that doesnt make sense at all, you should get a channel only when you
 > want to use it and not in probe!

 Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Every driver I ever looked at does.  Name one you consider "correct."
>
> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> it does request DMA channels at open time, not at probe time.

In the part quoted above, I said most drivers request dma channels in
their probe or open functions.  For the purposes of this discussion,
that distinction is irrelevant.  In either case, the channel is held
indefinitely.  If this wasn't the correct way to use the dmaengine,
there would be no need for the virt-dma helpers which are specifically
designed for cases the one currently at hand.

The only problem we have is that nobody envisioned hardware where the
dma engine indicates completion slightly too soon.  I suspect there's a
fifo or such somewhere, and the interrupt is triggered when the last
byte has been placed in the fifo rather than when it has been removed
which would have been more correct.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
>> Vinod Koul  writes:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
 Vinod Koul  writes:
 > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
 >> That's not going to work very well.  Device drivers typically request
 >> dma channels in their probe functions or when the device is opened.
 >> This means that reserving one of the few channels there will inevitably
 >> make some other device fail to operate.
 >
 > No that doesnt make sense at all, you should get a channel only when you
 > want to use it and not in probe!

 Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Every driver I ever looked at does.  Name one you consider "correct."
>
> I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but
> it does request DMA channels at open time, not at probe time.

In the part quoted above, I said most drivers request dma channels in
their probe or open functions.  For the purposes of this discussion,
that distinction is irrelevant.  In either case, the channel is held
indefinitely.  If this wasn't the correct way to use the dmaengine,
there would be no need for the virt-dma helpers which are specifically
designed for cases the one currently at hand.

The only problem we have is that nobody envisioned hardware where the
dma engine indicates completion slightly too soon.  I suspect there's a
fifo or such somewhere, and the interrupt is triggered when the last
byte has been placed in the fifo rather than when it has been removed
which would have been more correct.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:03, Geert Uytterhoeven wrote:

> Måns Rullgård wrote:
>
>> Geert Uytterhoeven writes:
>>
>>> Can you fall back to PIO if requesting a channel fails?
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
> 
> You still want to fall back to PIO if the DMA engine is not available
> at all (e.g. DMA engine driver not compiled in, or module not loaded).

FWIW, the ECC engine is tied to the DMA engine. So PIO means
not only taking a hit from tying up the CPU for a slooow
transfer, but also a huge hit if ECC must be computed in SW.
(A 100x perf degradation is not unlikely.)

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 13:03, Geert Uytterhoeven wrote:

> Måns Rullgård wrote:
>
>> Geert Uytterhoeven writes:
>>
>>> Can you fall back to PIO if requesting a channel fails?
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
> 
> You still want to fall back to PIO if the DMA engine is not available
> at all (e.g. DMA engine driver not compiled in, or module not loaded).

FWIW, the ECC engine is tied to the DMA engine. So PIO means
not only taking a hit from tying up the CPU for a slooow
transfer, but also a huge hit if ECC must be computed in SW.
(A 100x perf degradation is not unlikely.)

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

Hi Måns,

On Thu, Dec 8, 2016 at 12:47 PM, Måns Rullgård  wrote:
> Geert Uytterhoeven  writes:
>> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
>>> On 08/12/2016 11:39, Vinod Koul wrote:
 On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>>> That's not going to work very well.  Device drivers typically request
>>> dma channels in their probe functions or when the device is opened.
>>> This means that reserving one of the few channels there will inevitably
>>> make some other device fail to operate.
>>
>> No that doesn't make sense at all, you should get a channel only when you
>> want to use it and not in probe!
>
> Tell that to just about every single driver ever written.

 Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Vinod,
>>>
>>> Could you explain something to me in layman's terms?
>>>
>>> I have a NAND Flash Controller driver that depends on the
>>> DMA driver under discussion.
>>>
>>> Suppose I move the dma_request_chan() call from the driver's
>>> probe function, to the actual DMA transfer function.
>>>
>>> I would want dma_request_chan() to put the calling thread
>>> to sleep until a channel becomes available (possibly with
>>> a timeout value).
>>>
>>> But Maxime told me dma_request_chan() will just return
>>> -EBUSY if no channels are available.
>>>
>>> Am I supposed to busy wait in my driver's DMA function
>>> until a channel becomes available?
>>
>> Can you fall back to PIO if requesting a channel fails?
>>
>> Alternatively, dma_request_chan() could always succeed, and
>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>> available due to a limitation on the number of active channels, and
>> the driver could fall back to PIO for that transfer.
>
> Why are we debating this nonsense?  There is an easy fix that doesn't
> require changing the semantics of existing functions or falling back to
> slow pio.

You still want to fall back to PIO if the DMA engine is not available at all
(e.g. DMA engine driver not compiled in, or module not loaded).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

Hi Måns,

On Thu, Dec 8, 2016 at 12:47 PM, Måns Rullgård  wrote:
> Geert Uytterhoeven  writes:
>> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
>>> On 08/12/2016 11:39, Vinod Koul wrote:
 On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>>> That's not going to work very well.  Device drivers typically request
>>> dma channels in their probe functions or when the device is opened.
>>> This means that reserving one of the few channels there will inevitably
>>> make some other device fail to operate.
>>
>> No that doesn't make sense at all, you should get a channel only when you
>> want to use it and not in probe!
>
> Tell that to just about every single driver ever written.

 Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Vinod,
>>>
>>> Could you explain something to me in layman's terms?
>>>
>>> I have a NAND Flash Controller driver that depends on the
>>> DMA driver under discussion.
>>>
>>> Suppose I move the dma_request_chan() call from the driver's
>>> probe function, to the actual DMA transfer function.
>>>
>>> I would want dma_request_chan() to put the calling thread
>>> to sleep until a channel becomes available (possibly with
>>> a timeout value).
>>>
>>> But Maxime told me dma_request_chan() will just return
>>> -EBUSY if no channels are available.
>>>
>>> Am I supposed to busy wait in my driver's DMA function
>>> until a channel becomes available?
>>
>> Can you fall back to PIO if requesting a channel fails?
>>
>> Alternatively, dma_request_chan() could always succeed, and
>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>> available due to a limitation on the number of active channels, and
>> the driver could fall back to PIO for that transfer.
>
> Why are we debating this nonsense?  There is an easy fix that doesn't
> require changing the semantics of existing functions or falling back to
> slow pio.

You still want to fall back to PIO if the DMA engine is not available at all
(e.g. DMA engine driver not compiled in, or module not loaded).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
> Vinod Koul  writes:
>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>>> >> That's not going to work very well.  Device drivers typically request
>>> >> dma channels in their probe functions or when the device is opened.
>>> >> This means that reserving one of the few channels there will inevitably
>>> >> make some other device fail to operate.
>>> >
>>> > No that doesnt make sense at all, you should get a channel only when you
>>> > want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Every driver I ever looked at does.  Name one you consider "correct."

I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but it does
request DMA channels at open time, not at probe time.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 12:44 PM, Måns Rullgård  wrote:
> Vinod Koul  writes:
>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
>>> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>>> >> That's not going to work very well.  Device drivers typically request
>>> >> dma channels in their probe functions or when the device is opened.
>>> >> This means that reserving one of the few channels there will inevitably
>>> >> make some other device fail to operate.
>>> >
>>> > No that doesnt make sense at all, you should get a channel only when you
>>> > want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Every driver I ever looked at does.  Name one you consider "correct."

I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but it does
request DMA channels at open time, not at probe time.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
>> On 08/12/2016 11:39, Vinod Koul wrote:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
 Vinod Koul  writes:
> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> No that doesn't make sense at all, you should get a channel only when you
> want to use it and not in probe!

 Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Vinod,
>>
>> Could you explain something to me in layman's terms?
>>
>> I have a NAND Flash Controller driver that depends on the
>> DMA driver under discussion.
>>
>> Suppose I move the dma_request_chan() call from the driver's
>> probe function, to the actual DMA transfer function.
>>
>> I would want dma_request_chan() to put the calling thread
>> to sleep until a channel becomes available (possibly with
>> a timeout value).
>>
>> But Maxime told me dma_request_chan() will just return
>> -EBUSY if no channels are available.
>>
>> Am I supposed to busy wait in my driver's DMA function
>> until a channel becomes available?
>
> Can you fall back to PIO if requesting a channel fails?
>
> Alternatively, dma_request_chan() could always succeed, and
> dmaengine_prep_slave_sg() could fail if the channel is currently not
> available due to a limitation on the number of active channels, and
> the driver could fall back to PIO for that transfer.

Why are we debating this nonsense?  There is an easy fix that doesn't
require changing the semantics of existing functions or falling back to
slow pio.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Geert Uytterhoeven  writes:

> On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
>> On 08/12/2016 11:39, Vinod Koul wrote:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
 Vinod Koul  writes:
> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> No that doesn't make sense at all, you should get a channel only when you
> want to use it and not in probe!

 Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Vinod,
>>
>> Could you explain something to me in layman's terms?
>>
>> I have a NAND Flash Controller driver that depends on the
>> DMA driver under discussion.
>>
>> Suppose I move the dma_request_chan() call from the driver's
>> probe function, to the actual DMA transfer function.
>>
>> I would want dma_request_chan() to put the calling thread
>> to sleep until a channel becomes available (possibly with
>> a timeout value).
>>
>> But Maxime told me dma_request_chan() will just return
>> -EBUSY if no channels are available.
>>
>> Am I supposed to busy wait in my driver's DMA function
>> until a channel becomes available?
>
> Can you fall back to PIO if requesting a channel fails?
>
> Alternatively, dma_request_chan() could always succeed, and
> dmaengine_prep_slave_sg() could fail if the channel is currently not
> available due to a limitation on the number of active channels, and
> the driver could fall back to PIO for that transfer.

Why are we debating this nonsense?  There is an easy fix that doesn't
require changing the semantics of existing functions or falling back to
slow pio.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> 
>> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> >> 
>> >> That's not going to work very well.  Device drivers typically request
>> >> dma channels in their probe functions or when the device is opened.
>> >> This means that reserving one of the few channels there will inevitably
>> >> make some other device fail to operate.
>> >
>> > No that doesnt make sense at all, you should get a channel only when you
>> > want to use it and not in probe!
>> 
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

Every driver I ever looked at does.  Name one you consider "correct."

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> 
>> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> >> 
>> >> That's not going to work very well.  Device drivers typically request
>> >> dma channels in their probe functions or when the device is opened.
>> >> This means that reserving one of the few channels there will inevitably
>> >> make some other device fail to operate.
>> >
>> > No that doesnt make sense at all, you should get a channel only when you
>> > want to use it and not in probe!
>> 
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

Every driver I ever looked at does.  Name one you consider "correct."

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Wed, Dec 07, 2016 at 04:44:55PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> >> 
>> >> What you're proposing, Vinod, is to make a channel exclusive
>> >> to a driver, as long as the driver has not explicitly released
>> >> the channel, via dma_release_channel(), right?
>> >
>> > Precisely, but yes the downside of that is concurrent access are
>> > limited, but am not sure if driver implements virtual channels and
>> > allows that..
>> 
>> My driver implements virtual channels.  The problem is that the physical
>> dma channels signal completion slightly too soon, at least with
>> mem-to-device transfers.  Apparently we need to keep the sbox routing
>> until the peripheral indicates that it has actually received all the
>> data.
>
> So do we need concurrent accesses by all clients.

Depends on what people do with the chips.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Wed, Dec 07, 2016 at 04:44:55PM +, Måns Rullgård wrote:
>> Vinod Koul  writes:
>> >> 
>> >> What you're proposing, Vinod, is to make a channel exclusive
>> >> to a driver, as long as the driver has not explicitly released
>> >> the channel, via dma_release_channel(), right?
>> >
>> > Precisely, but yes the downside of that is concurrent access are
>> > limited, but am not sure if driver implements virtual channels and
>> > allows that..
>> 
>> My driver implements virtual channels.  The problem is that the physical
>> dma channels signal completion slightly too soon, at least with
>> mem-to-device transfers.  Apparently we need to keep the sbox routing
>> until the peripheral indicates that it has actually received all the
>> data.
>
> So do we need concurrent accesses by all clients.

Depends on what people do with the chips.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
 On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

 No that doesn't make sense at all, you should get a channel only when you
 want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Vinod,
>
> Could you explain something to me in layman's terms?
>
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
>
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
>
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
>
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.
>
> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

Can you fall back to PIO if requesting a channel fails?

Alternatively, dma_request_chan() could always succeed, and
dmaengine_prep_slave_sg() could fail if the channel is currently not
available due to a limitation on the number of active channels, and
the driver could fall back to PIO for that transfer.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Geert Uytterhoeven]

On Thu, Dec 8, 2016 at 11:54 AM, Mason  wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
>> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>>> Vinod Koul  writes:
 On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

 No that doesn't make sense at all, you should get a channel only when you
 want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Vinod,
>
> Could you explain something to me in layman's terms?
>
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
>
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
>
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
>
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.
>
> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

Can you fall back to PIO if requesting a channel fails?

Alternatively, dma_request_chan() could always succeed, and
dmaengine_prep_slave_sg() could fail if the channel is currently not
available due to a limitation on the number of active channels, and
the driver could fall back to PIO for that transfer.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 11:39, Vinod Koul wrote:

> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>
>> Vinod Koul  writes:
>>
>>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:

 That's not going to work very well.  Device drivers typically request
 dma channels in their probe functions or when the device is opened.
 This means that reserving one of the few channels there will inevitably
 make some other device fail to operate.
>>>
>>> No that doesn't make sense at all, you should get a channel only when you
>>> want to use it and not in probe!
>>
>> Tell that to just about every single driver ever written.
> 
> Not really, few do yes which is wrong but not _all_ do that.

Vinod,

Could you explain something to me in layman's terms?

I have a NAND Flash Controller driver that depends on the
DMA driver under discussion.

Suppose I move the dma_request_chan() call from the driver's
probe function, to the actual DMA transfer function.

I would want dma_request_chan() to put the calling thread
to sleep until a channel becomes available (possibly with
a timeout value).

But Maxime told me dma_request_chan() will just return
-EBUSY if no channels are available.

Am I supposed to busy wait in my driver's DMA function
until a channel becomes available?

I don't understand how the multiplexing of few memory
channels to many clients is supposed to happen efficiently?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 08/12/2016 11:39, Vinod Koul wrote:

> On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
>
>> Vinod Koul  writes:
>>
>>> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:

 That's not going to work very well.  Device drivers typically request
 dma channels in their probe functions or when the device is opened.
 This means that reserving one of the few channels there will inevitably
 make some other device fail to operate.
>>>
>>> No that doesn't make sense at all, you should get a channel only when you
>>> want to use it and not in probe!
>>
>> Tell that to just about every single driver ever written.
> 
> Not really, few do yes which is wrong but not _all_ do that.

Vinod,

Could you explain something to me in layman's terms?

I have a NAND Flash Controller driver that depends on the
DMA driver under discussion.

Suppose I move the dma_request_chan() call from the driver's
probe function, to the actual DMA transfer function.

I would want dma_request_chan() to put the calling thread
to sleep until a channel becomes available (possibly with
a timeout value).

But Maxime told me dma_request_chan() will just return
-EBUSY if no channels are available.

Am I supposed to busy wait in my driver's DMA function
until a channel becomes available?

I don't understand how the multiplexing of few memory
channels to many clients is supposed to happen efficiently?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> 
> >> That's not going to work very well.  Device drivers typically request
> >> dma channels in their probe functions or when the device is opened.
> >> This means that reserving one of the few channels there will inevitably
> >> make some other device fail to operate.
> >
> > No that doesnt make sense at all, you should get a channel only when you
> > want to use it and not in probe!
> 
> Tell that to just about every single driver ever written.

Not really, few do yes which is wrong but not _all_ do that.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Wed, Dec 07, 2016 at 04:45:58PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> 
> > On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> >> 
> >> That's not going to work very well.  Device drivers typically request
> >> dma channels in their probe functions or when the device is opened.
> >> This means that reserving one of the few channels there will inevitably
> >> make some other device fail to operate.
> >
> > No that doesnt make sense at all, you should get a channel only when you
> > want to use it and not in probe!
> 
> Tell that to just about every single driver ever written.

Not really, few do yes which is wrong but not _all_ do that.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Wed, Dec 07, 2016 at 04:44:55PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> >> 
> >> What you're proposing, Vinod, is to make a channel exclusive
> >> to a driver, as long as the driver has not explicitly released
> >> the channel, via dma_release_channel(), right?
> >
> > Precisely, but yes the downside of that is concurrent access are
> > limited, but am not sure if driver implements virtual channels and
> > allows that..
> 
> My driver implements virtual channels.  The problem is that the physical
> dma channels signal completion slightly too soon, at least with
> mem-to-device transfers.  Apparently we need to keep the sbox routing
> until the peripheral indicates that it has actually received all the
> data.

So do we need concurrent accesses by all clients.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Wed, Dec 07, 2016 at 04:44:55PM +, Måns Rullgård wrote:
> Vinod Koul  writes:
> >> 
> >> What you're proposing, Vinod, is to make a channel exclusive
> >> to a driver, as long as the driver has not explicitly released
> >> the channel, via dma_release_channel(), right?
> >
> > Precisely, but yes the downside of that is concurrent access are
> > limited, but am not sure if driver implements virtual channels and
> > allows that..
> 
> My driver implements virtual channels.  The problem is that the physical
> dma channels signal completion slightly too soon, at least with
> mem-to-device transfers.  Apparently we need to keep the sbox routing
> until the peripheral indicates that it has actually received all the
> data.

So do we need concurrent accesses by all clients.

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> 
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> No that doesnt make sense at all, you should get a channel only when you
> want to use it and not in probe!

Tell that to just about every single driver ever written.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
>> 
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> No that doesnt make sense at all, you should get a channel only when you
> want to use it and not in probe!

Tell that to just about every single driver ever written.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
>> On 06/12/2016 06:12, Vinod Koul wrote:
>> 
>> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> > 
>> >> Is there a way to write a driver within the existing framework?
>> > 
>> > I think so, looking back at comments from Russell, I do tend to agree with
>> > that. Is there a specific reason why sbox can't be tied to alloc and free
>> > channels?
>> 
>> Here's a recap of the situation.
>> 
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> btw is SBOX setup dependent upon the peripheral connected to?

The sbox is basically a crossbar that connects each of a number of input
ports to any of a number of output ports.  A few of the inputs and
outputs are dma channels reading or writing to memory while the rest are
peripheral devices.  To perform a mem-to-device transfer, you pick a dma
read channel, program the sbox to connect it to the chosen device, and
finally program the dma channel with address and size to transfer.

>> tango3
>>   2 memory channels available
>>   6 devices ("clients"?) may request an MBUS channel
>
> But only 2 can get a channel at any time..
>
>> tango4 (one more channel)
>>   3 memory channels available
>>   7 devices may request an MBUS channel :
>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Same here
>
> Only thing is users shouldn't hold on to channel and freeup when not in use.
>
>> Notes:
>> The current NFC driver supports only one controller.
>> IDE is mostly obsolete at this point.
>> 
>> tango5 (SATA gets own dedicated MBUS channel pair)
>>   3 memory channels available
>>   5 devices may request an MBUS channel :
>> NFC0, NFC1, memcpy, (IDE0, IDE1)
>> 
>> 
>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel. The DMA driver
>> manages a per-channel queue of outstanding DMA transfer requests,
>> and a new transfer is started friom within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion
>> of the transfer, as explained else-thread).
>> 
>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
>
> Precisely, but yes the downside of that is concurrent access are
> limited, but am not sure if driver implements virtual channels and
> allows that..

My driver implements virtual channels.  The problem is that the physical
dma channels signal completion slightly too soon, at least with
mem-to-device transfers.  Apparently we need to keep the sbox routing
until the peripheral indicates that it has actually received all the
data.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Vinod Koul  writes:

> On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
>> On 06/12/2016 06:12, Vinod Koul wrote:
>> 
>> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> > 
>> >> Is there a way to write a driver within the existing framework?
>> > 
>> > I think so, looking back at comments from Russell, I do tend to agree with
>> > that. Is there a specific reason why sbox can't be tied to alloc and free
>> > channels?
>> 
>> Here's a recap of the situation.
>> 
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> btw is SBOX setup dependent upon the peripheral connected to?

The sbox is basically a crossbar that connects each of a number of input
ports to any of a number of output ports.  A few of the inputs and
outputs are dma channels reading or writing to memory while the rest are
peripheral devices.  To perform a mem-to-device transfer, you pick a dma
read channel, program the sbox to connect it to the chosen device, and
finally program the dma channel with address and size to transfer.

>> tango3
>>   2 memory channels available
>>   6 devices ("clients"?) may request an MBUS channel
>
> But only 2 can get a channel at any time..
>
>> tango4 (one more channel)
>>   3 memory channels available
>>   7 devices may request an MBUS channel :
>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Same here
>
> Only thing is users shouldn't hold on to channel and freeup when not in use.
>
>> Notes:
>> The current NFC driver supports only one controller.
>> IDE is mostly obsolete at this point.
>> 
>> tango5 (SATA gets own dedicated MBUS channel pair)
>>   3 memory channels available
>>   5 devices may request an MBUS channel :
>> NFC0, NFC1, memcpy, (IDE0, IDE1)
>> 
>> 
>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel. The DMA driver
>> manages a per-channel queue of outstanding DMA transfer requests,
>> and a new transfer is started friom within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion
>> of the transfer, as explained else-thread).
>> 
>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
>
> Precisely, but yes the downside of that is concurrent access are
> limited, but am not sure if driver implements virtual channels and
> allows that..

My driver implements virtual channels.  The problem is that the physical
dma channels signal completion slightly too soon, at least with
mem-to-device transfers.  Apparently we need to keep the sbox routing
until the peripheral indicates that it has actually received all the
data.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
> On 06/12/2016 06:12, Vinod Koul wrote:
> 
> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> > 
> >> Is there a way to write a driver within the existing framework?
> > 
> > I think so, looking back at comments from Russell, I do tend to agree with
> > that. Is there a specific reason why sbox can't be tied to alloc and free
> > channels?
> 
> Here's a recap of the situation.
> 
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:

btw is SBOX setup dependent upon the peripheral connected to?

> 
> tango3
>   2 memory channels available
>   6 devices ("clients"?) may request an MBUS channel

But only 2 can get a channel at any time..

> 
> tango4 (one more channel)
>   3 memory channels available
>   7 devices may request an MBUS channel :
> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)

Same here

Only thing is users shouldn't hold on to channel and freeup when not in use.

> Notes:
> The current NFC driver supports only one controller.
> IDE is mostly obsolete at this point.
> 
> tango5 (SATA gets own dedicated MBUS channel pair)
>   3 memory channels available
>   5 devices may request an MBUS channel :
> NFC0, NFC1, memcpy, (IDE0, IDE1)
> 
> 
> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel. The DMA driver
> manages a per-channel queue of outstanding DMA transfer requests,
> and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion
> of the transfer, as explained else-thread).
> 
> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?

Precisely, but yes the downside of that is concurrent access are limited, but
am not sure if driver implements virtual channels and allows that..

Thanks
-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Dec 06, 2016 at 01:42:31PM +0100, Mason wrote:
> On 06/12/2016 06:12, Vinod Koul wrote:
> 
> > On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> > 
> >> Is there a way to write a driver within the existing framework?
> > 
> > I think so, looking back at comments from Russell, I do tend to agree with
> > that. Is there a specific reason why sbox can't be tied to alloc and free
> > channels?
> 
> Here's a recap of the situation.
> 
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:

btw is SBOX setup dependent upon the peripheral connected to?

> 
> tango3
>   2 memory channels available
>   6 devices ("clients"?) may request an MBUS channel

But only 2 can get a channel at any time..

> 
> tango4 (one more channel)
>   3 memory channels available
>   7 devices may request an MBUS channel :
> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)

Same here

Only thing is users shouldn't hold on to channel and freeup when not in use.

> Notes:
> The current NFC driver supports only one controller.
> IDE is mostly obsolete at this point.
> 
> tango5 (SATA gets own dedicated MBUS channel pair)
>   3 memory channels available
>   5 devices may request an MBUS channel :
> NFC0, NFC1, memcpy, (IDE0, IDE1)
> 
> 
> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel. The DMA driver
> manages a per-channel queue of outstanding DMA transfer requests,
> and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion
> of the transfer, as explained else-thread).
> 
> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?

Precisely, but yes the downside of that is concurrent access are limited, but
am not sure if driver implements virtual channels and allows that..

Thanks
-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> 
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

No that doesnt make sense at all, you should get a channel only when you
want to use it and not in probe!

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Dec 06, 2016 at 01:14:20PM +, Måns Rullgård wrote:
> 
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

No that doesnt make sense at all, you should get a channel only when you
want to use it and not in probe!

-- 
~Vinod

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 16:34, Måns Rullgård wrote:

> Mason writes:
> 
>> Meh. The two controller blocks share the I/O pins to the outside
>> world, so it's not possible to have two concurrent accesses.
> 
> OK, you failed to mention that part.  Why are there two controllers at
> all if only one or the other can be used?

I'd have to ask the HW designer what types of use-cases he had
in mind. Perhaps looking at what is *not* shared provides clues.
Configuration registers are duplicated, meaning it is possible
to decide "channel A for chip0, channel B for chip1" if there
are two NAND chips in the system (as is the case on dev boards).
In that case, it is unnecessary to rewrite the chip parameters
every time the driver switches chips. (I don't think the perf
impact is even measurable.)

The ECC engines are duplicated, but I don't know how long it
takes to run the BCH algorithm in HW vs performing I/O over
a slow 8-bit bus.


>> The callback is called from vchan_complete() right?
>> Is that running from interrupt context?
> 
> It runs from a tasklet which is almost the same thing.

I'll read up on tasklets tomorrow.


>> I can give that a shot (if you're busy with real work).
> 
> I have an idea I'd like to try out over the weekend.  If I don't come
> back with something by next week, go for it.

I do have my plate full this week, with XHCI and AHCI :-)


>> Why can't we queue channel requests the same way we queue
>> transfer requests?
> 
> That's in effect what we're doing.  Calling it by another name doesn't
> really solve anything.

Hmmm... the difference is that "tear down" would be explicit
in the release function.

Current implementation for single transfer:

dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()   /* A */
wait for read channel IRQ   /* B */
spin until NFC idle /* C */

Setup SBOX route and program MBUS transfer happen in A.
The SBOX route is torn down a little before B (thus before C).


With the proposed implementation, where request_chan
sets up the route and release_chan tears it down:

request_chan()  /* X */
dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()   /* A */
wait for read channel IRQ   /* B */
spin until NFC idle /* C */
release_chan()  /* Y */

Now, the SBOX route is setup in X.
(If no MBUS channel are available, thread is put to sleep
until one becomes available.)
Program MBUS transfer in A.
When IRQ falls, cannot start new transfer yet.
vchan_complete() will at some point run the client callback.
The client driver can now spin however long until NFC idle.
In Y, we release the channel, thus calling back into the
DMA driver, at which point a new transfer can be started.

What did I get wrong in this pseudo-code?


I do see one problem with the approach:
It's no big deal for me to convert the NFC driver to
do it that way, but the Synopsys (I think) SATA driver
in tango3 and tango4 is shared across multiple SoCs,
and they probably call request_chan() only in the
probe function, as you mentioned at some point.

http://lxr.free-electrons.com/source/drivers/ata/sata_dwc_460ex.c#L366

BTW, can someone explain what the DMA_CTRL_ACK flag means?


Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 16:34, Måns Rullgård wrote:

> Mason writes:
> 
>> Meh. The two controller blocks share the I/O pins to the outside
>> world, so it's not possible to have two concurrent accesses.
> 
> OK, you failed to mention that part.  Why are there two controllers at
> all if only one or the other can be used?

I'd have to ask the HW designer what types of use-cases he had
in mind. Perhaps looking at what is *not* shared provides clues.
Configuration registers are duplicated, meaning it is possible
to decide "channel A for chip0, channel B for chip1" if there
are two NAND chips in the system (as is the case on dev boards).
In that case, it is unnecessary to rewrite the chip parameters
every time the driver switches chips. (I don't think the perf
impact is even measurable.)

The ECC engines are duplicated, but I don't know how long it
takes to run the BCH algorithm in HW vs performing I/O over
a slow 8-bit bus.


>> The callback is called from vchan_complete() right?
>> Is that running from interrupt context?
> 
> It runs from a tasklet which is almost the same thing.

I'll read up on tasklets tomorrow.


>> I can give that a shot (if you're busy with real work).
> 
> I have an idea I'd like to try out over the weekend.  If I don't come
> back with something by next week, go for it.

I do have my plate full this week, with XHCI and AHCI :-)


>> Why can't we queue channel requests the same way we queue
>> transfer requests?
> 
> That's in effect what we're doing.  Calling it by another name doesn't
> really solve anything.

Hmmm... the difference is that "tear down" would be explicit
in the release function.

Current implementation for single transfer:

dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()   /* A */
wait for read channel IRQ   /* B */
spin until NFC idle /* C */

Setup SBOX route and program MBUS transfer happen in A.
The SBOX route is torn down a little before B (thus before C).


With the proposed implementation, where request_chan
sets up the route and release_chan tears it down:

request_chan()  /* X */
dmaengine_prep_slave_sg()
dmaengine_submit()
dma_async_issue_pending()   /* A */
wait for read channel IRQ   /* B */
spin until NFC idle /* C */
release_chan()  /* Y */

Now, the SBOX route is setup in X.
(If no MBUS channel are available, thread is put to sleep
until one becomes available.)
Program MBUS transfer in A.
When IRQ falls, cannot start new transfer yet.
vchan_complete() will at some point run the client callback.
The client driver can now spin however long until NFC idle.
In Y, we release the channel, thus calling back into the
DMA driver, at which point a new transfer can be started.

What did I get wrong in this pseudo-code?


I do see one problem with the approach:
It's no big deal for me to convert the NFC driver to
do it that way, but the Synopsys (I think) SATA driver
in tango3 and tango4 is shared across multiple SoCs,
and they probably call request_chan() only in the
probe function, as you mentioned at some point.

http://lxr.free-electrons.com/source/drivers/ata/sata_dwc_460ex.c#L366

BTW, can someone explain what the DMA_CTRL_ACK flag means?


Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 06/12/2016 14:14, Måns Rullgård wrote:
>
>> Mason wrote:
>> 
>>> On 06/12/2016 06:12, Vinod Koul wrote:
>>>
 On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:

> Is there a way to write a driver within the existing framework?

 I think so, looking back at comments from Russell, I do tend to agree with
 that. Is there a specific reason why sbox can't be tied to alloc and free
 channels?
>>>
>>> Here's a recap of the situation.
>>>
>>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>>
>>> tango3
>>>   2 memory channels available
>>>   6 devices ("clients"?) may request an MBUS channel
>>>
>>> tango4 (one more channel)
>>>   3 memory channels available
>>>   7 devices may request an MBUS channel :
>>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>>
>>> Notes:
>>> The current NFC driver supports only one controller.
>> 
>> I consider that a bug.
>
> Meh. The two controller blocks share the I/O pins to the outside
> world, so it's not possible to have two concurrent accesses.

OK, you failed to mention that part.  Why are there two controllers at
all if only one or the other can be used?

>>> If I understand the current DMA driver (written by Mans), client
>>> drivers are instructed to use a specific channel in the DT, and
>>> the DMA driver muxes access to that channel.
>> 
>> Almost.  The DT indicates the sbox ID of each device.  The driver
>> multiplexes requests from all devices across all channels.
>
> Thanks for pointing that out. I misremembered the DT.
> So a client's DT node specifies the client's SBOX port.
> And the DMA node specifies all available MBUS channels.
>
> So when an interrupt fires, the DMA driver (re)uses that
> channel for the next transfer in line?

Correct.

>>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>>> requests, and a new transfer is started from within the DMA ISR
>>> (modulo the fact that the interrupt does not signal completion of the
>>> transfer, as explained else-thread).
>> 
>> We need to somehow let the device driver signal the dma driver when a
>> transfer has been fully completed.  Currently the only post-transfer
>> interaction between the dma engine and the device driver is through the
>> descriptor callback, which is not suitable for this purpose.
>
> The callback is called from vchan_complete() right?
> Is that running from interrupt context?

It runs from a tasklet which is almost the same thing.

> What's the relationship between vchan_complete() and
> tangox_dma_irq() -- does one call the other? Are they
> asynchronous?
>
>> This is starting to look like one of those situations where someone just
>> needs to implement a solution, or we'll be forever bickering about
>> hypotheticals.
>
> I can give that a shot (if you're busy with real work).

I have an idea I'd like to try out over the weekend.  If I don't come
back with something by next week, go for it.

>>> What you're proposing, Vinod, is to make a channel exclusive
>>> to a driver, as long as the driver has not explicitly released
>>> the channel, via dma_release_channel(), right?
>> 
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> This is true for tango3. Less so for tango4. And no longer
> an issue for tango5.
>
>> Doing a request/release per transfer really doesn't fit with the
>> intended usage of the dmaengine api.  For starters, what should a driver
>> do if all the channels are currently busy?
>
> Why can't we queue channel requests the same way we queue
> transfer requests?

That's in effect what we're doing.  Calling it by another name doesn't
really solve anything.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 06/12/2016 14:14, Måns Rullgård wrote:
>
>> Mason wrote:
>> 
>>> On 06/12/2016 06:12, Vinod Koul wrote:
>>>
 On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:

> Is there a way to write a driver within the existing framework?

 I think so, looking back at comments from Russell, I do tend to agree with
 that. Is there a specific reason why sbox can't be tied to alloc and free
 channels?
>>>
>>> Here's a recap of the situation.
>>>
>>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>>
>>> tango3
>>>   2 memory channels available
>>>   6 devices ("clients"?) may request an MBUS channel
>>>
>>> tango4 (one more channel)
>>>   3 memory channels available
>>>   7 devices may request an MBUS channel :
>>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>>
>>> Notes:
>>> The current NFC driver supports only one controller.
>> 
>> I consider that a bug.
>
> Meh. The two controller blocks share the I/O pins to the outside
> world, so it's not possible to have two concurrent accesses.

OK, you failed to mention that part.  Why are there two controllers at
all if only one or the other can be used?

>>> If I understand the current DMA driver (written by Mans), client
>>> drivers are instructed to use a specific channel in the DT, and
>>> the DMA driver muxes access to that channel.
>> 
>> Almost.  The DT indicates the sbox ID of each device.  The driver
>> multiplexes requests from all devices across all channels.
>
> Thanks for pointing that out. I misremembered the DT.
> So a client's DT node specifies the client's SBOX port.
> And the DMA node specifies all available MBUS channels.
>
> So when an interrupt fires, the DMA driver (re)uses that
> channel for the next transfer in line?

Correct.

>>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>>> requests, and a new transfer is started from within the DMA ISR
>>> (modulo the fact that the interrupt does not signal completion of the
>>> transfer, as explained else-thread).
>> 
>> We need to somehow let the device driver signal the dma driver when a
>> transfer has been fully completed.  Currently the only post-transfer
>> interaction between the dma engine and the device driver is through the
>> descriptor callback, which is not suitable for this purpose.
>
> The callback is called from vchan_complete() right?
> Is that running from interrupt context?

It runs from a tasklet which is almost the same thing.

> What's the relationship between vchan_complete() and
> tangox_dma_irq() -- does one call the other? Are they
> asynchronous?
>
>> This is starting to look like one of those situations where someone just
>> needs to implement a solution, or we'll be forever bickering about
>> hypotheticals.
>
> I can give that a shot (if you're busy with real work).

I have an idea I'd like to try out over the weekend.  If I don't come
back with something by next week, go for it.

>>> What you're proposing, Vinod, is to make a channel exclusive
>>> to a driver, as long as the driver has not explicitly released
>>> the channel, via dma_release_channel(), right?
>> 
>> That's not going to work very well.  Device drivers typically request
>> dma channels in their probe functions or when the device is opened.
>> This means that reserving one of the few channels there will inevitably
>> make some other device fail to operate.
>
> This is true for tango3. Less so for tango4. And no longer
> an issue for tango5.
>
>> Doing a request/release per transfer really doesn't fit with the
>> intended usage of the dmaengine api.  For starters, what should a driver
>> do if all the channels are currently busy?
>
> Why can't we queue channel requests the same way we queue
> transfer requests?

That's in effect what we're doing.  Calling it by another name doesn't
really solve anything.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 14:14, Måns Rullgård wrote:

> Mason wrote:
> 
>> On 06/12/2016 06:12, Vinod Koul wrote:
>>
>>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>>>
 Is there a way to write a driver within the existing framework?
>>>
>>> I think so, looking back at comments from Russell, I do tend to agree with
>>> that. Is there a specific reason why sbox can't be tied to alloc and free
>>> channels?
>>
>> Here's a recap of the situation.
>>
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>
>> tango3
>>   2 memory channels available
>>   6 devices ("clients"?) may request an MBUS channel
>>
>> tango4 (one more channel)
>>   3 memory channels available
>>   7 devices may request an MBUS channel :
>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>
>> Notes:
>> The current NFC driver supports only one controller.
> 
> I consider that a bug.

Meh. The two controller blocks share the I/O pins to the outside
world, so it's not possible to have two concurrent accesses.
Moreover, the current NAND framework does not currently support
such a setup. (I discussed this with the maintainer.)


>> IDE is mostly obsolete at this point.
>>
>> tango5 (SATA gets own dedicated MBUS channel pair)
>>   3 memory channels available
>>   5 devices may request an MBUS channel :
>> NFC0, NFC1, memcpy, (IDE0, IDE1)
> 
> Some of the chip variants can also use this DMA engine for PCI devices.

Note: PCI support was dropped with tango4.


>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel.
> 
> Almost.  The DT indicates the sbox ID of each device.  The driver
> multiplexes requests from all devices across all channels.

Thanks for pointing that out. I misremembered the DT.
So a client's DT node specifies the client's SBOX port.
And the DMA node specifies all available MBUS channels.

So when an interrupt fires, the DMA driver (re)uses that
channel for the next transfer in line?


>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>> requests, and a new transfer is started from within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion of the
>> transfer, as explained else-thread).
> 
> We need to somehow let the device driver signal the dma driver when a
> transfer has been fully completed.  Currently the only post-transfer
> interaction between the dma engine and the device driver is through the
> descriptor callback, which is not suitable for this purpose.

The callback is called from vchan_complete() right?
Is that running from interrupt context?

What's the relationship between vchan_complete() and
tangox_dma_irq() -- does one call the other? Are they
asynchronous?


> This is starting to look like one of those situations where someone just
> needs to implement a solution, or we'll be forever bickering about
> hypotheticals.

I can give that a shot (if you're busy with real work).


>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
> 
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

This is true for tango3. Less so for tango4. And no longer
an issue for tango5.


> Doing a request/release per transfer really doesn't fit with the
> intended usage of the dmaengine api.  For starters, what should a driver
> do if all the channels are currently busy?

Why can't we queue channel requests the same way we queue
transfer requests?


> Since the hardware actually does support multiplexing the dma channels,
> I think it would be misguided to deliberately cripple the software
> support in order to shoehorn it into an incomplete model of how hardware
> ought to work.  While I agree it would be nicer if all hardware actually
> did work that way, this isn't the reality we're living in.

I agree with you that it would be nice to have a general solution,
since the HW supports it.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 14:14, Måns Rullgård wrote:

> Mason wrote:
> 
>> On 06/12/2016 06:12, Vinod Koul wrote:
>>
>>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>>>
 Is there a way to write a driver within the existing framework?
>>>
>>> I think so, looking back at comments from Russell, I do tend to agree with
>>> that. Is there a specific reason why sbox can't be tied to alloc and free
>>> channels?
>>
>> Here's a recap of the situation.
>>
>> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>>
>> tango3
>>   2 memory channels available
>>   6 devices ("clients"?) may request an MBUS channel
>>
>> tango4 (one more channel)
>>   3 memory channels available
>>   7 devices may request an MBUS channel :
>> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>>
>> Notes:
>> The current NFC driver supports only one controller.
> 
> I consider that a bug.

Meh. The two controller blocks share the I/O pins to the outside
world, so it's not possible to have two concurrent accesses.
Moreover, the current NAND framework does not currently support
such a setup. (I discussed this with the maintainer.)


>> IDE is mostly obsolete at this point.
>>
>> tango5 (SATA gets own dedicated MBUS channel pair)
>>   3 memory channels available
>>   5 devices may request an MBUS channel :
>> NFC0, NFC1, memcpy, (IDE0, IDE1)
> 
> Some of the chip variants can also use this DMA engine for PCI devices.

Note: PCI support was dropped with tango4.


>> If I understand the current DMA driver (written by Mans), client
>> drivers are instructed to use a specific channel in the DT, and
>> the DMA driver muxes access to that channel.
> 
> Almost.  The DT indicates the sbox ID of each device.  The driver
> multiplexes requests from all devices across all channels.

Thanks for pointing that out. I misremembered the DT.
So a client's DT node specifies the client's SBOX port.
And the DMA node specifies all available MBUS channels.

So when an interrupt fires, the DMA driver (re)uses that
channel for the next transfer in line?


>> The DMA driver manages a per-channel queue of outstanding DMA transfer
>> requests, and a new transfer is started from within the DMA ISR
>> (modulo the fact that the interrupt does not signal completion of the
>> transfer, as explained else-thread).
> 
> We need to somehow let the device driver signal the dma driver when a
> transfer has been fully completed.  Currently the only post-transfer
> interaction between the dma engine and the device driver is through the
> descriptor callback, which is not suitable for this purpose.

The callback is called from vchan_complete() right?
Is that running from interrupt context?

What's the relationship between vchan_complete() and
tangox_dma_irq() -- does one call the other? Are they
asynchronous?


> This is starting to look like one of those situations where someone just
> needs to implement a solution, or we'll be forever bickering about
> hypotheticals.

I can give that a shot (if you're busy with real work).


>> What you're proposing, Vinod, is to make a channel exclusive
>> to a driver, as long as the driver has not explicitly released
>> the channel, via dma_release_channel(), right?
> 
> That's not going to work very well.  Device drivers typically request
> dma channels in their probe functions or when the device is opened.
> This means that reserving one of the few channels there will inevitably
> make some other device fail to operate.

This is true for tango3. Less so for tango4. And no longer
an issue for tango5.


> Doing a request/release per transfer really doesn't fit with the
> intended usage of the dmaengine api.  For starters, what should a driver
> do if all the channels are currently busy?

Why can't we queue channel requests the same way we queue
transfer requests?


> Since the hardware actually does support multiplexing the dma channels,
> I think it would be misguided to deliberately cripple the software
> support in order to shoehorn it into an incomplete model of how hardware
> ought to work.  While I agree it would be nicer if all hardware actually
> did work that way, this isn't the reality we're living in.

I agree with you that it would be nice to have a general solution,
since the HW supports it.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 06/12/2016 06:12, Vinod Koul wrote:
>
>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> 
>>> Is there a way to write a driver within the existing framework?
>> 
>> I think so, looking back at comments from Russell, I do tend to agree with
>> that. Is there a specific reason why sbox can't be tied to alloc and free
>> channels?
>
> Here's a recap of the situation.
>
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> tango3
>   2 memory channels available
>   6 devices ("clients"?) may request an MBUS channel
>
> tango4 (one more channel)
>   3 memory channels available
>   7 devices may request an MBUS channel :
> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Notes:
> The current NFC driver supports only one controller.

I consider that a bug.

> IDE is mostly obsolete at this point.
>
> tango5 (SATA gets own dedicated MBUS channel pair)
>   3 memory channels available
>   5 devices may request an MBUS channel :
> NFC0, NFC1, memcpy, (IDE0, IDE1)

Some of the chip variants can also use this DMA engine for PCI devices.

> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel.

Almost.  The DT indicates the sbox ID of each device.  The driver
multiplexes requests from all devices across all channels.

> The DMA driver manages a per-channel queue of outstanding DMA transfer
> requests, and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion of the
> transfer, as explained else-thread).

We need to somehow let the device driver signal the dma driver when a
transfer has been fully completed.  Currently the only post-transfer
interaction between the dma engine and the device driver is through the
descriptor callback, which is not suitable for this purpose.

This is starting to look like one of those situations where someone just
needs to implement a solution, or we'll be forever bickering about
hypotheticals.

> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?

That's not going to work very well.  Device drivers typically request
dma channels in their probe functions or when the device is opened.
This means that reserving one of the few channels there will inevitably
make some other device fail to operate.

Doing a request/release per transfer really doesn't fit with the
intended usage of the dmaengine api.  For starters, what should a driver
do if all the channels are currently busy?

Since the hardware actually does support multiplexing the dma channels,
I think it would be misguided to deliberately cripple the software
support in order to shoehorn it into an incomplete model of how hardware
ought to work.  While I agree it would be nicer if all hardware actually
did work that way, this isn't the reality we're living in.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Måns Rullgård]

Mason  writes:

> On 06/12/2016 06:12, Vinod Koul wrote:
>
>> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
>> 
>>> Is there a way to write a driver within the existing framework?
>> 
>> I think so, looking back at comments from Russell, I do tend to agree with
>> that. Is there a specific reason why sbox can't be tied to alloc and free
>> channels?
>
> Here's a recap of the situation.
>
> The "SBOX+MBUS" HW is used in several iterations of the tango SoC:
>
> tango3
>   2 memory channels available
>   6 devices ("clients"?) may request an MBUS channel
>
> tango4 (one more channel)
>   3 memory channels available
>   7 devices may request an MBUS channel :
> NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)
>
> Notes:
> The current NFC driver supports only one controller.

I consider that a bug.

> IDE is mostly obsolete at this point.
>
> tango5 (SATA gets own dedicated MBUS channel pair)
>   3 memory channels available
>   5 devices may request an MBUS channel :
> NFC0, NFC1, memcpy, (IDE0, IDE1)

Some of the chip variants can also use this DMA engine for PCI devices.

> If I understand the current DMA driver (written by Mans), client
> drivers are instructed to use a specific channel in the DT, and
> the DMA driver muxes access to that channel.

Almost.  The DT indicates the sbox ID of each device.  The driver
multiplexes requests from all devices across all channels.

> The DMA driver manages a per-channel queue of outstanding DMA transfer
> requests, and a new transfer is started friom within the DMA ISR
> (modulo the fact that the interrupt does not signal completion of the
> transfer, as explained else-thread).

We need to somehow let the device driver signal the dma driver when a
transfer has been fully completed.  Currently the only post-transfer
interaction between the dma engine and the device driver is through the
descriptor callback, which is not suitable for this purpose.

This is starting to look like one of those situations where someone just
needs to implement a solution, or we'll be forever bickering about
hypotheticals.

> What you're proposing, Vinod, is to make a channel exclusive
> to a driver, as long as the driver has not explicitly released
> the channel, via dma_release_channel(), right?

That's not going to work very well.  Device drivers typically request
dma channels in their probe functions or when the device is opened.
This means that reserving one of the few channels there will inevitably
make some other device fail to operate.

Doing a request/release per transfer really doesn't fit with the
intended usage of the dmaengine api.  For starters, what should a driver
do if all the channels are currently busy?

Since the hardware actually does support multiplexing the dma channels,
I think it would be misguided to deliberately cripple the software
support in order to shoehorn it into an incomplete model of how hardware
ought to work.  While I agree it would be nicer if all hardware actually
did work that way, this isn't the reality we're living in.

-- 
Måns Rullgård

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 06:12, Vinod Koul wrote:

> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> 
>> Is there a way to write a driver within the existing framework?
> 
> I think so, looking back at comments from Russell, I do tend to agree with
> that. Is there a specific reason why sbox can't be tied to alloc and free
> channels?

Here's a recap of the situation.

The "SBOX+MBUS" HW is used in several iterations of the tango SoC:

tango3
  2 memory channels available
  6 devices ("clients"?) may request an MBUS channel

tango4 (one more channel)
  3 memory channels available
  7 devices may request an MBUS channel :
NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)

Notes:
The current NFC driver supports only one controller.
IDE is mostly obsolete at this point.

tango5 (SATA gets own dedicated MBUS channel pair)
  3 memory channels available
  5 devices may request an MBUS channel :
NFC0, NFC1, memcpy, (IDE0, IDE1)


If I understand the current DMA driver (written by Mans), client
drivers are instructed to use a specific channel in the DT, and
the DMA driver muxes access to that channel. The DMA driver
manages a per-channel queue of outstanding DMA transfer requests,
and a new transfer is started friom within the DMA ISR
(modulo the fact that the interrupt does not signal completion
of the transfer, as explained else-thread).

What you're proposing, Vinod, is to make a channel exclusive
to a driver, as long as the driver has not explicitly released
the channel, via dma_release_channel(), right?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 06/12/2016 06:12, Vinod Koul wrote:

> On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:
> 
>> Is there a way to write a driver within the existing framework?
> 
> I think so, looking back at comments from Russell, I do tend to agree with
> that. Is there a specific reason why sbox can't be tied to alloc and free
> channels?

Here's a recap of the situation.

The "SBOX+MBUS" HW is used in several iterations of the tango SoC:

tango3
  2 memory channels available
  6 devices ("clients"?) may request an MBUS channel

tango4 (one more channel)
  3 memory channels available
  7 devices may request an MBUS channel :
NFC0, NFC1, SATA0, SATA1, memcpy, (IDE0, IDE1)

Notes:
The current NFC driver supports only one controller.
IDE is mostly obsolete at this point.

tango5 (SATA gets own dedicated MBUS channel pair)
  3 memory channels available
  5 devices may request an MBUS channel :
NFC0, NFC1, memcpy, (IDE0, IDE1)


If I understand the current DMA driver (written by Mans), client
drivers are instructed to use a specific channel in the DT, and
the DMA driver muxes access to that channel. The DMA driver
manages a per-channel queue of outstanding DMA transfer requests,
and a new transfer is started friom within the DMA ISR
(modulo the fact that the interrupt does not signal completion
of the transfer, as explained else-thread).

What you're proposing, Vinod, is to make a channel exclusive
to a driver, as long as the driver has not explicitly released
the channel, via dma_release_channel(), right?

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:

Sorry I was away for a week in meeting with laptop down.

> [ Nothing new added below.
>   Vinod, was the description of my HW's quirks clear enough?

Yes

>   Is there a way to write a driver within the existing framework?

I think so, looking back at comments from Russell, I do tend to agree with
that. Is there a specfic reason why sbox can't be tied to alloc and free
channels?

>   How can I get that HW block supported upstream?
>   Regards. ]
> 
> On 25/11/2016 13:46, Mason wrote:
> 
> > On 25/11/2016 05:55, Vinod Koul wrote:
> > 
> >> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
> >>
> >>> On my platform, setting up a DMA transfer is a two-step process:
> >>>
> >>> 1) configure the "switch box" to connect a device to a memory channel
> >>> 2) configure the transfer details (address, size, command)
> >>>
> >>> When the transfer is done, the sbox setup can be torn down,
> >>> and the DMA driver can start another transfer.
> >>>
> >>> The current software architecture for my NFC (NAND Flash controller)
> >>> driver is as follows (for one DMA transfer).
> >>>
> >>>   sg_init_one
> >>>   dma_map_sg
> >>>   dmaengine_prep_slave_sg
> >>>   dmaengine_submit
> >>>   dma_async_issue_pending
> >>>   configure_NFC_transfer
> >>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
> >>>   wait_for_NFC_idle
> >>>   dma_unmap_sg
> >>
> >> Looking at thread and discussion now, first thinking would be to ensure
> >> the transaction is completed properly and then isr fired. You may need
> >> to talk to your HW designers to find a way for that. It is quite common
> >> that DMA controllers will fire and complete whereas the transaction is
> >> still in flight.
> > 
> > It seems there is a disconnect between what Linux expects - an IRQ
> > when the transfer is complete - and the quirks of this HW :-(
> > 
> > On this system, there are MBUS "agents" connected via a "switch box".
> > An agent fires an IRQ when it has dealt with its *half* of the transfer.
> > 
> > SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> > 
> > Here are the steps for a transfer, in the general case:
> > 
> > 1) setup the sbox to connect SOURCE TO DEST
> > 2) configure source to send N bytes
> > 3) configure dest to receive N bytes
> > 
> > When SOURCE_AGENT has sent N bytes, it fires an IRQ
> > When DEST_AGENT has received N bytes, it fires an IRQ
> > The sbox connection can be torn down only when the destination
> > agent has received all bytes.
> > (And the twist is that some agents do not have an IRQ line.)
> > 
> > The system provides 3 RAM-to-sbox agents (read channels)
> > and 3 sbox-to-RAM agents (write channels).
> > 
> > The NAND Flash controller read and write agents do not have
> > IRQ lines.
> > 
> > So for a NAND-to-memory transfer (read from device)
> > - nothing happens when the NFC has finished sending N bytes to the sbox
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > In that case, one IRQ fires when the transfer is complete,
> > like Linux expects.
> > 
> > For a memory-to-NAND transfer (write to device)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the NFC driver is supposed to poll the NFC to determine
> > when the controller has finished writing N bytes
> > 
> > In that case, the IRQ does not indicate that the transfer
> > is complete, merely that the sending half has finished
> > its part.
> > 
> > For a memory-to-memory transfer (memcpy)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > So you actually get two IRQs in that case, which I don't
> > think Linux (or the current DMA driver) expects.
> > 
> > I'm not sure how we're supposed to handle this kind of HW
> > in Linux? (That's why I started this thread.)
> > 
> > 
> >> If that is not doable, then since you claim this is custom part which
> >> other vendors won't use (hope we are wrong down the line),
> > 
> > I'm not sure how to interpret "you claim this is custom part".
> > Do you mean I may be wrong, that it is not custom?
> > I don't know if other vendors may have HW with the same
> > quirky behavior. What do you mean about being wrong down
> > the line?
> > 
> >> then we can have a custom api,
> >>
> >> foo_sbox_configure(bool enable, ...);
> >>
> >> This can be invoked from NFC driver when required for configuration and
> >> teardown. For very specific cases where people need some specific
> >> configuration we do allow custom APIs.
> > 
> > I don't think that would work. The fundamental issue is
> > that Linux expects a single IRQ to indicate "transfer
> > complete". And the driver (as written) starts a new
> > transfer as soon as the IRQ fires.
> > 
> > But the HW may generate 0, 1, or even 2 IRQs for a single
> > transfer. And when there is a single IRQ, it may not
> > indicate "transfer complete" (as seen above).
> > 
> >> Only problem with that would be it 

Re: Tearing down DMA transfer setup after DMA client has finished

[Vinod Koul]

On Tue, Nov 29, 2016 at 07:25:02PM +0100, Mason wrote:

Sorry I was away for a week in meeting with laptop down.

> [ Nothing new added below.
>   Vinod, was the description of my HW's quirks clear enough?

Yes

>   Is there a way to write a driver within the existing framework?

I think so, looking back at comments from Russell, I do tend to agree with
that. Is there a specfic reason why sbox can't be tied to alloc and free
channels?

>   How can I get that HW block supported upstream?
>   Regards. ]
> 
> On 25/11/2016 13:46, Mason wrote:
> 
> > On 25/11/2016 05:55, Vinod Koul wrote:
> > 
> >> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
> >>
> >>> On my platform, setting up a DMA transfer is a two-step process:
> >>>
> >>> 1) configure the "switch box" to connect a device to a memory channel
> >>> 2) configure the transfer details (address, size, command)
> >>>
> >>> When the transfer is done, the sbox setup can be torn down,
> >>> and the DMA driver can start another transfer.
> >>>
> >>> The current software architecture for my NFC (NAND Flash controller)
> >>> driver is as follows (for one DMA transfer).
> >>>
> >>>   sg_init_one
> >>>   dma_map_sg
> >>>   dmaengine_prep_slave_sg
> >>>   dmaengine_submit
> >>>   dma_async_issue_pending
> >>>   configure_NFC_transfer
> >>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
> >>>   wait_for_NFC_idle
> >>>   dma_unmap_sg
> >>
> >> Looking at thread and discussion now, first thinking would be to ensure
> >> the transaction is completed properly and then isr fired. You may need
> >> to talk to your HW designers to find a way for that. It is quite common
> >> that DMA controllers will fire and complete whereas the transaction is
> >> still in flight.
> > 
> > It seems there is a disconnect between what Linux expects - an IRQ
> > when the transfer is complete - and the quirks of this HW :-(
> > 
> > On this system, there are MBUS "agents" connected via a "switch box".
> > An agent fires an IRQ when it has dealt with its *half* of the transfer.
> > 
> > SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> > 
> > Here are the steps for a transfer, in the general case:
> > 
> > 1) setup the sbox to connect SOURCE TO DEST
> > 2) configure source to send N bytes
> > 3) configure dest to receive N bytes
> > 
> > When SOURCE_AGENT has sent N bytes, it fires an IRQ
> > When DEST_AGENT has received N bytes, it fires an IRQ
> > The sbox connection can be torn down only when the destination
> > agent has received all bytes.
> > (And the twist is that some agents do not have an IRQ line.)
> > 
> > The system provides 3 RAM-to-sbox agents (read channels)
> > and 3 sbox-to-RAM agents (write channels).
> > 
> > The NAND Flash controller read and write agents do not have
> > IRQ lines.
> > 
> > So for a NAND-to-memory transfer (read from device)
> > - nothing happens when the NFC has finished sending N bytes to the sbox
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > In that case, one IRQ fires when the transfer is complete,
> > like Linux expects.
> > 
> > For a memory-to-NAND transfer (write to device)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the NFC driver is supposed to poll the NFC to determine
> > when the controller has finished writing N bytes
> > 
> > In that case, the IRQ does not indicate that the transfer
> > is complete, merely that the sending half has finished
> > its part.
> > 
> > For a memory-to-memory transfer (memcpy)
> > - the read channel fires an IRQ when it has sent N bytes
> > - the write channel fires an IRQ when it has received N bytes
> > 
> > So you actually get two IRQs in that case, which I don't
> > think Linux (or the current DMA driver) expects.
> > 
> > I'm not sure how we're supposed to handle this kind of HW
> > in Linux? (That's why I started this thread.)
> > 
> > 
> >> If that is not doable, then since you claim this is custom part which
> >> other vendors won't use (hope we are wrong down the line),
> > 
> > I'm not sure how to interpret "you claim this is custom part".
> > Do you mean I may be wrong, that it is not custom?
> > I don't know if other vendors may have HW with the same
> > quirky behavior. What do you mean about being wrong down
> > the line?
> > 
> >> then we can have a custom api,
> >>
> >> foo_sbox_configure(bool enable, ...);
> >>
> >> This can be invoked from NFC driver when required for configuration and
> >> teardown. For very specific cases where people need some specific
> >> configuration we do allow custom APIs.
> > 
> > I don't think that would work. The fundamental issue is
> > that Linux expects a single IRQ to indicate "transfer
> > complete". And the driver (as written) starts a new
> > transfer as soon as the IRQ fires.
> > 
> > But the HW may generate 0, 1, or even 2 IRQs for a single
> > transfer. And when there is a single IRQ, it may not
> > indicate "transfer complete" (as seen above).
> > 
> >> Only problem with that would be it 

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

[ Nothing new added below.
  Vinod, was the description of my HW's quirks clear enough?
  Is there a way to write a driver within the existing framework?
  How can I get that HW block supported upstream?
  Regards. ]

On 25/11/2016 13:46, Mason wrote:

> On 25/11/2016 05:55, Vinod Koul wrote:
> 
>> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>>
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   dma_unmap_sg
>>
>> Looking at thread and discussion now, first thinking would be to ensure
>> the transaction is completed properly and then isr fired. You may need
>> to talk to your HW designers to find a way for that. It is quite common
>> that DMA controllers will fire and complete whereas the transaction is
>> still in flight.
> 
> It seems there is a disconnect between what Linux expects - an IRQ
> when the transfer is complete - and the quirks of this HW :-(
> 
> On this system, there are MBUS "agents" connected via a "switch box".
> An agent fires an IRQ when it has dealt with its *half* of the transfer.
> 
> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> 
> Here are the steps for a transfer, in the general case:
> 
> 1) setup the sbox to connect SOURCE TO DEST
> 2) configure source to send N bytes
> 3) configure dest to receive N bytes
> 
> When SOURCE_AGENT has sent N bytes, it fires an IRQ
> When DEST_AGENT has received N bytes, it fires an IRQ
> The sbox connection can be torn down only when the destination
> agent has received all bytes.
> (And the twist is that some agents do not have an IRQ line.)
> 
> The system provides 3 RAM-to-sbox agents (read channels)
> and 3 sbox-to-RAM agents (write channels).
> 
> The NAND Flash controller read and write agents do not have
> IRQ lines.
> 
> So for a NAND-to-memory transfer (read from device)
> - nothing happens when the NFC has finished sending N bytes to the sbox
> - the write channel fires an IRQ when it has received N bytes
> 
> In that case, one IRQ fires when the transfer is complete,
> like Linux expects.
> 
> For a memory-to-NAND transfer (write to device)
> - the read channel fires an IRQ when it has sent N bytes
> - the NFC driver is supposed to poll the NFC to determine
> when the controller has finished writing N bytes
> 
> In that case, the IRQ does not indicate that the transfer
> is complete, merely that the sending half has finished
> its part.
> 
> For a memory-to-memory transfer (memcpy)
> - the read channel fires an IRQ when it has sent N bytes
> - the write channel fires an IRQ when it has received N bytes
> 
> So you actually get two IRQs in that case, which I don't
> think Linux (or the current DMA driver) expects.
> 
> I'm not sure how we're supposed to handle this kind of HW
> in Linux? (That's why I started this thread.)
> 
> 
>> If that is not doable, then since you claim this is custom part which
>> other vendors won't use (hope we are wrong down the line),
> 
> I'm not sure how to interpret "you claim this is custom part".
> Do you mean I may be wrong, that it is not custom?
> I don't know if other vendors may have HW with the same
> quirky behavior. What do you mean about being wrong down
> the line?
> 
>> then we can have a custom api,
>>
>> foo_sbox_configure(bool enable, ...);
>>
>> This can be invoked from NFC driver when required for configuration and
>> teardown. For very specific cases where people need some specific
>> configuration we do allow custom APIs.
> 
> I don't think that would work. The fundamental issue is
> that Linux expects a single IRQ to indicate "transfer
> complete". And the driver (as written) starts a new
> transfer as soon as the IRQ fires.
> 
> But the HW may generate 0, 1, or even 2 IRQs for a single
> transfer. And when there is a single IRQ, it may not
> indicate "transfer complete" (as seen above).
> 
>> Only problem with that would be it wont be a generic solution
>> and you seem to be fine with that.
> 
> I think it is possible to have a generic solution:
> Right now, the callback is called from tasklet context.
> If we can have a new flag to have the callback invoked
> directly from the ISR, then the driver for the client
> device can do what is required.
> 
> For example, the NFC driver waits for the IRQ from the
> memory agent, and then polls the controller itself.
> 
> I can whip up a proof-of-concept if it's better to
> illustrate with a 

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

[ Nothing new added below.
  Vinod, was the description of my HW's quirks clear enough?
  Is there a way to write a driver within the existing framework?
  How can I get that HW block supported upstream?
  Regards. ]

On 25/11/2016 13:46, Mason wrote:

> On 25/11/2016 05:55, Vinod Koul wrote:
> 
>> On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
>>
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   dma_unmap_sg
>>
>> Looking at thread and discussion now, first thinking would be to ensure
>> the transaction is completed properly and then isr fired. You may need
>> to talk to your HW designers to find a way for that. It is quite common
>> that DMA controllers will fire and complete whereas the transaction is
>> still in flight.
> 
> It seems there is a disconnect between what Linux expects - an IRQ
> when the transfer is complete - and the quirks of this HW :-(
> 
> On this system, there are MBUS "agents" connected via a "switch box".
> An agent fires an IRQ when it has dealt with its *half* of the transfer.
> 
> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
> 
> Here are the steps for a transfer, in the general case:
> 
> 1) setup the sbox to connect SOURCE TO DEST
> 2) configure source to send N bytes
> 3) configure dest to receive N bytes
> 
> When SOURCE_AGENT has sent N bytes, it fires an IRQ
> When DEST_AGENT has received N bytes, it fires an IRQ
> The sbox connection can be torn down only when the destination
> agent has received all bytes.
> (And the twist is that some agents do not have an IRQ line.)
> 
> The system provides 3 RAM-to-sbox agents (read channels)
> and 3 sbox-to-RAM agents (write channels).
> 
> The NAND Flash controller read and write agents do not have
> IRQ lines.
> 
> So for a NAND-to-memory transfer (read from device)
> - nothing happens when the NFC has finished sending N bytes to the sbox
> - the write channel fires an IRQ when it has received N bytes
> 
> In that case, one IRQ fires when the transfer is complete,
> like Linux expects.
> 
> For a memory-to-NAND transfer (write to device)
> - the read channel fires an IRQ when it has sent N bytes
> - the NFC driver is supposed to poll the NFC to determine
> when the controller has finished writing N bytes
> 
> In that case, the IRQ does not indicate that the transfer
> is complete, merely that the sending half has finished
> its part.
> 
> For a memory-to-memory transfer (memcpy)
> - the read channel fires an IRQ when it has sent N bytes
> - the write channel fires an IRQ when it has received N bytes
> 
> So you actually get two IRQs in that case, which I don't
> think Linux (or the current DMA driver) expects.
> 
> I'm not sure how we're supposed to handle this kind of HW
> in Linux? (That's why I started this thread.)
> 
> 
>> If that is not doable, then since you claim this is custom part which
>> other vendors won't use (hope we are wrong down the line),
> 
> I'm not sure how to interpret "you claim this is custom part".
> Do you mean I may be wrong, that it is not custom?
> I don't know if other vendors may have HW with the same
> quirky behavior. What do you mean about being wrong down
> the line?
> 
>> then we can have a custom api,
>>
>> foo_sbox_configure(bool enable, ...);
>>
>> This can be invoked from NFC driver when required for configuration and
>> teardown. For very specific cases where people need some specific
>> configuration we do allow custom APIs.
> 
> I don't think that would work. The fundamental issue is
> that Linux expects a single IRQ to indicate "transfer
> complete". And the driver (as written) starts a new
> transfer as soon as the IRQ fires.
> 
> But the HW may generate 0, 1, or even 2 IRQs for a single
> transfer. And when there is a single IRQ, it may not
> indicate "transfer complete" (as seen above).
> 
>> Only problem with that would be it wont be a generic solution
>> and you seem to be fine with that.
> 
> I think it is possible to have a generic solution:
> Right now, the callback is called from tasklet context.
> If we can have a new flag to have the callback invoked
> directly from the ISR, then the driver for the client
> device can do what is required.
> 
> For example, the NFC driver waits for the IRQ from the
> memory agent, and then polls the controller itself.
> 
> I can whip up a proof-of-concept if it's better to
> illustrate with a 

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 25/11/2016 15:37, Måns Rullgård wrote:

> Mason writes:
> 
>> On 25/11/2016 14:11, Måns Rullgård wrote:
>>
>>> Mason writes:
>>>
 It seems there is a disconnect between what Linux expects - an IRQ
 when the transfer is complete - and the quirks of this HW :-(

 On this system, there are MBUS "agents" connected via a "switch box".
 An agent fires an IRQ when it has dealt with its *half* of the transfer.

 SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT

 Here are the steps for a transfer, in the general case:

 1) setup the sbox to connect SOURCE TO DEST
 2) configure source to send N bytes
 3) configure dest to receive N bytes

 When SOURCE_AGENT has sent N bytes, it fires an IRQ
 When DEST_AGENT has received N bytes, it fires an IRQ
 The sbox connection can be torn down only when the destination
 agent has received all bytes.
 (And the twist is that some agents do not have an IRQ line.)

 The system provides 3 RAM-to-sbox agents (read channels)
 and 3 sbox-to-RAM agents (write channels).

 The NAND Flash controller read and write agents do not have
 IRQ lines.

 So for a NAND-to-memory transfer (read from device)
 - nothing happens when the NFC has finished sending N bytes to the sbox
 - the write channel fires an IRQ when it has received N bytes

 In that case, one IRQ fires when the transfer is complete,
 like Linux expects.

 For a memory-to-NAND transfer (write to device)
 - the read channel fires an IRQ when it has sent N bytes
 - the NFC driver is supposed to poll the NFC to determine
 when the controller has finished writing N bytes

 In that case, the IRQ does not indicate that the transfer
 is complete, merely that the sending half has finished
 its part.
>>>
>>> When does your NAND controller signal completion?  When it has received
>>> the DMA data, or only when it has finished the actual write operation?
>>
>> The NAND controller provides a STATUS register.
>> Bit 31 is the CMD_READY bit.
>> This bit goes to 0 when the controller is busy, and to 1
>> when the controller is ready to accept the next command.
>>
>> The NFC driver is doing:
>>
>>  res = wait_for_completion_timeout(_done, HZ);
>>  if (res > 0)
>>  err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
>>
>> So basically, sleep until the memory agent IRQ falls,
>> then spin until the controller is idle.
> 
> This doesn't answer my question.  Waiting for the entire operation to
> finish isn't necessary.  The dma driver only needs to wait until all the
> data has been received by the nand controller, not until the controller
> is completely finished with the command.  Does the nand controller
> provide an indication for completion of the dma independently of the
> progress of the write command?  The dma glue Sigma added to the
> Designware sata controller does this.

I called the HW dev. He told me the NFC block does not have
buffers to store the incoming data; so they remain in the
MBUS FIFOs until the NFC consumes them, i.e. when it has
finished writing them to a NAND chip, which could take
a "long time" when writing to a slow chip.

So the answer to your question is: "the NAND controller
signals completion only when it has finished the actual
write operation."

>> Did you see that adding a 10 µs delay at the start of
>> tangox_dma_pchan_detach() makes the system no longer
>> fail (passes an mtd_speedtest).
> 
> Yes, but maybe that's much longer than is actually necessary.

I could instrument my spin loop to record how long we had
to wait between the IRQ and CMD_READY.

Regards.

Re: Tearing down DMA transfer setup after DMA client has finished

[Mason]

On 25/11/2016 15:37, Måns Rullgård wrote:

> Mason writes:
> 
>> On 25/11/2016 14:11, Måns Rullgård wrote:
>>
>>> Mason writes:
>>>
 It seems there is a disconnect between what Linux expects - an IRQ
 when the transfer is complete - and the quirks of this HW :-(

 On this system, there are MBUS "agents" connected via a "switch box".
 An agent fires an IRQ when it has dealt with its *half* of the transfer.

 SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT

 Here are the steps for a transfer, in the general case:

 1) setup the sbox to connect SOURCE TO DEST
 2) configure source to send N bytes
 3) configure dest to receive N bytes

 When SOURCE_AGENT has sent N bytes, it fires an IRQ
 When DEST_AGENT has received N bytes, it fires an IRQ
 The sbox connection can be torn down only when the destination
 agent has received all bytes.
 (And the twist is that some agents do not have an IRQ line.)

 The system provides 3 RAM-to-sbox agents (read channels)
 and 3 sbox-to-RAM agents (write channels).

 The NAND Flash controller read and write agents do not have
 IRQ lines.

 So for a NAND-to-memory transfer (read from device)
 - nothing happens when the NFC has finished sending N bytes to the sbox
 - the write channel fires an IRQ when it has received N bytes

 In that case, one IRQ fires when the transfer is complete,
 like Linux expects.

 For a memory-to-NAND transfer (write to device)
 - the read channel fires an IRQ when it has sent N bytes
 - the NFC driver is supposed to poll the NFC to determine
 when the controller has finished writing N bytes

 In that case, the IRQ does not indicate that the transfer
 is complete, merely that the sending half has finished
 its part.
>>>
>>> When does your NAND controller signal completion?  When it has received
>>> the DMA data, or only when it has finished the actual write operation?
>>
>> The NAND controller provides a STATUS register.
>> Bit 31 is the CMD_READY bit.
>> This bit goes to 0 when the controller is busy, and to 1
>> when the controller is ready to accept the next command.
>>
>> The NFC driver is doing:
>>
>>  res = wait_for_completion_timeout(_done, HZ);
>>  if (res > 0)
>>  err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
>>
>> So basically, sleep until the memory agent IRQ falls,
>> then spin until the controller is idle.
> 
> This doesn't answer my question.  Waiting for the entire operation to
> finish isn't necessary.  The dma driver only needs to wait until all the
> data has been received by the nand controller, not until the controller
> is completely finished with the command.  Does the nand controller
> provide an indication for completion of the dma independently of the
> progress of the write command?  The dma glue Sigma added to the
> Designware sata controller does this.

I called the HW dev. He told me the NFC block does not have
buffers to store the incoming data; so they remain in the
MBUS FIFOs until the NFC consumes them, i.e. when it has
finished writing them to a NAND chip, which could take
a "long time" when writing to a slow chip.

So the answer to your question is: "the NAND controller
signals completion only when it has finished the actual
write operation."

>> Did you see that adding a 10 µs delay at the start of
>> tangox_dma_pchan_detach() makes the system no longer
>> fail (passes an mtd_speedtest).
> 
> Yes, but maybe that's much longer than is actually necessary.

I could instrument my spin loop to record how long we had
to wait between the IRQ and CMD_READY.

Regards.