Re: [PATCH] dma: edma: add device_slave_caps() support
From: Lars-Peter Clausen
Date: Wed Jul 24 2013 - 15:15:31 EST
On 07/24/2013 08:55 PM, Joel Fernandes wrote:
On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote:
On 07/24/2013 10:28 AM, Fernandes, Joel wrote:
On Jul 24, 2013, at 3:23 AM, "Lars-Peter Clausen" <lars@xxxxxxxxxx> wrote:
On 07/24/2013 10:11 AM, Joel Fernandes wrote:
On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote:
On 07/23/2013 06:43 PM, Joel Fernandes wrote:
Implement device_slave_caps(). EDMA has a limited number of slots.
Slave drivers such as omap_hsmmc will query the driver to make
sure they don't pass in more than these many scatter segments.
Signed-off-by: Joel Fernandes <joelf@xxxxxx>
---
Vinod, or Dan- If this patch looks ok, can you please merge in for
-rc cycle? This patch is required to fix MMC support on AM33xx. This
patch is blocking 3 other patches which fix various MMC things. Thanks!
Notes:
(1) this approach is temporary and only for -rc cycle to fix MMC on
AM335x. It will be replace by the RFC series in future kernels:
http://www.spinics.net/lists/arm-kernel/msg260094.html
(2) Patch depends Vinod's patch at:
http://permalink.gmane.org/gmane.linux.kernel/1525112
drivers/dma/edma.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 7222cbe..81d5429 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan)
spin_unlock_irqrestore(&echan->vchan.lock, flags);
}
+static inline int edma_slave_caps(struct dma_chan *chan,
+ struct dma_slave_caps *caps)
+{
+ caps->max_sg_nr = MAX_NR_SG;
Hm, what about the other fields?
Other fields are unused, the max segment size is supposed to be
calculated "given" the address width and burst size. Since these
can't be provided to get_caps, I have left it out for now.
See: https://lkml.org/lkml/2013/3/6/464
The PL330 driver is similar in this regard, the maximum segment size also
depends on address width and burst width. What I did for the get_slave_caps
implementation is to set it to the minimum maximum size. E.g. in you case
that should be SZ_64K - 1 (burstsize and addrwidth both set to 1).
So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something..
Yes. This is a limitation of the current slave_caps API. The maximum needs
to be the maximum for all possible configurations. A specific configuration
may allow a larger maximum. So we maybe have to extend the API to be able to
query the limits for a certain configuration. Not sure what the best way
would be to do that, either adding a config parameter to get_slave_caps or
to break it into two functions like you proposed one for the static
capabilities and one for the sg limits.
I am OK with either approach as long as a decision can be made quickly
by maintainers. Right now lot of back and forth has happened and 3
different versions of the same thing have been posted since January.
Since this is such a trivial change, it doesn't make sense to spend so
much time on it IMO.... The sad part is though this change is trivial,
other drivers such as MMC are broken and cannot be enabled due to this.
We cannot afford to leave them broken.
Well this is a new API, so it is kind of expected that there is some back and
forth and that there will be a few revisions.
If Vinod is not available, can Dan please respond on how to proceed on
this? We really need this trivial change to go into this -rc cycle and
not delay it by another kernel release. Thank you.
This is not something you'd merge for rc3 or even later. If the MMC driver does
not work without this I guess it never worked, so strictly speaking there is no
regression and it is just a new feature.
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/