public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter@hurleysoftware.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	John Ogness <john.ogness@linutronix.de>,
	gregkh@linuxfoundation.org, vinod.koul@intel.com
Cc: dan.j.williams@intel.com, tony@atomide.com, nsekhar@ti.com,
	peter.ujfalusi@ti.com, dmaengine@vger.kernel.org,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] tty: serial: 8250: omap: convert to using cyclic transfers
Date: Sat, 23 Jan 2016 13:24:30 -0800	[thread overview]
Message-ID: <56A3EF8E.6020204@hurleysoftware.com> (raw)
In-Reply-To: <56A3DF17.2070908@linutronix.de>

On 01/23/2016 12:14 PM, Sebastian Andrzej Siewior wrote:
> On 01/22/2016 05:31 PM, Peter Hurley wrote:
>> Hi John,
>>
>> On 01/22/2016 02:27 AM, John Ogness wrote:
>>> Note that an ugly device-tree lookup hack is implemented in order
>>> to detect if the sDMA engine is being used. This is necessary
>>> because the sDMA driver does not implement pause/resume correctly
>>> and therefore requires slightly different logic for the UART
>>> driver.
>>
>> No, this is not acceptable.
>>
>> Please work with Vinod to refine the dma_get_slave_caps() interface
>> so that dmaengine drivers that don't provide the _documented_ pause/resume
>> support can be identified generically.
> 
> But we could implement the pause/resume bits correctly wait until it is
> merge and post this and rely on the fact that it works (including error
> checking). RMK requested that he wants the UART driver fixed before
> touching the DMA driver and this has been done in meantime. So we could
> go ahead and add the missing pieces to SDMA.

There is zero chance of backporting this.
Let's move forward and fix the problem.

At a minimum, the reason this code needs to know which DMA slave driver
is because the sDMA driver lies about pause support. Rather than continuing
to hide this broken behavior behind the dmaengine API, expose a method
of generically determining what the slave driver actually does.
It's the only sane choice.

Regards,
Peter Hurley

FWIW, I'm not convinced about cyclic transfers. But doing a complete review
of that is a bit off.

      reply	other threads:[~2016-01-23 21:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22 10:27 [PATCH 3/4] tty: serial: 8250: omap: convert to using cyclic transfers John Ogness
2016-01-22 16:31 ` Peter Hurley
2016-01-23 20:14   ` Sebastian Andrzej Siewior
2016-01-23 21:24     ` Peter Hurley [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A3EF8E.6020204@hurleysoftware.com \
    --to=peter@hurleysoftware.com \
    --cc=bigeasy@linutronix.de \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=nsekhar@ti.com \
    --cc=peter.ujfalusi@ti.com \
    --cc=tony@atomide.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox