Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1408994 > unrolled thread

Re: [PATCH 5/5] RFC spi: sun4i: add DMA support

Started byMark Brown <broonie@kernel.org>
First post2016-05-30 13:30 +0200
Last post2016-05-30 18:00 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 5/5] RFC spi: sun4i: add DMA support Mark Brown <broonie@kernel.org> - 2016-05-30 13:30 +0200
    Re: [PATCH 5/5] RFC spi: sun4i: add DMA support Geert Uytterhoeven <geert@linux-m68k.org> - 2016-05-30 14:20 +0200
      Re: [PATCH 5/5] RFC spi: sun4i: add DMA support Mark Brown <broonie@kernel.org> - 2016-05-30 17:10 +0200
        Re: [PATCH 5/5] RFC spi: sun4i: add DMA support Michal Suchanek <hramrach@gmail.com> - 2016-05-30 17:30 +0200
          Re: [PATCH 5/5] RFC spi: sun4i: add DMA support Mark Brown <broonie@kernel.org> - 2016-05-30 18:00 +0200

#1408994 — Re: [PATCH 5/5] RFC spi: sun4i: add DMA support

FromMark Brown <broonie@kernel.org>
Date2016-05-30 13:30 +0200
SubjectRe: [PATCH 5/5] RFC spi: sun4i: add DMA support
Message-ID<rEtyq-3wi-19@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:

>  - fallback to previous behaviour when DMA initialization fails
> 
>    + this has the problem that when the driver happens to load before the dma
>      driver it will not use dma - can be addressed with a module parameter

No, you should pay attention to the error you are getting and let probe
deferral happen if that's the error you get.

[toc] | [next] | [standalone]


#1409037

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-05-30 14:20 +0200
Message-ID<rEukN-44d-1@gated-at.bofh.it>
In reply to#1408994
Hi Mark,

On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
>>  - fallback to previous behaviour when DMA initialization fails
>>
>>    + this has the problem that when the driver happens to load before the dma
>>      driver it will not use dma - can be addressed with a module parameter
>
> No, you should pay attention to the error you are getting and let probe
> deferral happen if that's the error you get.

Unfortunately DMA is an optional feature.

There's no way to distinguish between -EPROBE_DEFER due to the SPI master
driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
support for the DMA engine not having been compiled in.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

[toc] | [prev] | [next] | [standalone]


#1409205

FromMark Brown <broonie@kernel.org>
Date2016-05-30 17:10 +0200
Message-ID<rEwZk-5PN-41@gated-at.bofh.it>
In reply to#1409037

[Multipart message — attachments visible in raw view] — view raw

On Mon, May 30, 2016 at 02:11:51PM +0200, Geert Uytterhoeven wrote:
> On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
> >>  - fallback to previous behaviour when DMA initialization fails
> >>
> >>    + this has the problem that when the driver happens to load before the dma
> >>      driver it will not use dma - can be addressed with a module parameter

> > No, you should pay attention to the error you are getting and let probe
> > deferral happen if that's the error you get.

> Unfortunately DMA is an optional feature.

> There's no way to distinguish between -EPROBE_DEFER due to the SPI master
> driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
> support for the DMA engine not having been compiled in.

I really don't think it's worth caring too much about cases where the
DMA driver hasn't been compiled in, it's not like SPI is the only thing
that's going to be using it.  I really think it's better to defer the
problem - not getting DMA (or worse, only getting DMA on some boots) is
not great and if people are optimising on that level my feeling is that
they're probably going to be OK with customizing DT to match.  Ideally
we would have something a bit nicer than deferred probe but right now
this seems the more helpful option.

[toc] | [prev] | [next] | [standalone]


#1409228

FromMichal Suchanek <hramrach@gmail.com>
Date2016-05-30 17:30 +0200
Message-ID<rExiF-5ZF-7@gated-at.bofh.it>
In reply to#1409205
Hello,

On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 30, 2016 at 02:11:51PM +0200, Geert Uytterhoeven wrote:
>> On Mon, May 30, 2016 at 1:26 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Thu, May 26, 2016 at 07:25:25PM -0000, Michal Suchanek wrote:
>> >>  - fallback to previous behaviour when DMA initialization fails
>> >>
>> >>    + this has the problem that when the driver happens to load before the dma
>> >>      driver it will not use dma - can be addressed with a module parameter
>
>> > No, you should pay attention to the error you are getting and let probe
>> > deferral happen if that's the error you get.
>
>> Unfortunately DMA is an optional feature.
>
>> There's no way to distinguish between -EPROBE_DEFER due to the SPI master
>> driver being probed before the DMA engine driver, and -EPROBE_DEFER due to
>> support for the DMA engine not having been compiled in.
>
> I really don't think it's worth caring too much about cases where the
> DMA driver hasn't been compiled in, it's not like SPI is the only thing
> that's going to be using it.  I really think it's better to defer the
> problem - not getting DMA (or worse, only getting DMA on some boots) is
> not great and if people are optimising on that level my feeling is that
> they're probably going to be OK with customizing DT to match.  Ideally
> we would have something a bit nicer than deferred probe but right now
> this seems the more helpful option.

It's what the driver did to start with and it was requested to fall
back to non-DMA in the case DMA is not available.

We get to the much discussed problem that it's never possible to tell
if a driver is available or not.

It's possible to add a parameter like require_dma which could be used
to load the driver without dma if unset. If it was set by default then
driver ordering is not important so long as dma driver is loaded
eventually. Also an informative print that such parameter exists when
probing the driver is deferred would be helpful. It would probably
create quite a bit of log spam, however. The driver can be deferred
several times during boot.

Thanks

Michal

[toc] | [prev] | [next] | [standalone]


#1409248

FromMark Brown <broonie@kernel.org>
Date2016-05-30 18:00 +0200
Message-ID<rExLI-6b9-11@gated-at.bofh.it>
In reply to#1409228

[Multipart message — attachments visible in raw view] — view raw

On Mon, May 30, 2016 at 05:28:10PM +0200, Michal Suchanek wrote:
> On 30 May 2016 at 17:03, Mark Brown <broonie@kernel.org> wrote:

> > I really don't think it's worth caring too much about cases where the
> > DMA driver hasn't been compiled in, it's not like SPI is the only thing

> It's what the driver did to start with and it was requested to fall
> back to non-DMA in the case DMA is not available.

Why?  I really can't see any sensible use case for this that doesn't
have a better solution available.

> It's possible to add a parameter like require_dma which could be used
> to load the driver without dma if unset. If it was set by default then
> driver ordering is not important so long as dma driver is loaded
> eventually. Also an informative print that such parameter exists when
> probing the driver is deferred would be helpful. It would probably
> create quite a bit of log spam, however. The driver can be deferred
> several times during boot.

That seems fairly hacky, if we were going to do anything like that it
should be the other way around so that we default to trying to use
resources and even then it seems like something that should be handled
at a framework level rather than having random options in individual
drivers to ignore things.  Having things behave inconsistently between
different drivers is going to lead to a worse user experience and if
this is a good idea for one driver it seems like it'd be a good idea for
all of them.

But really 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web