| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 23 Jul 2013 01:35:10 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Guennadi Liakhovetski <g.liakhovetski@....de>
Cc: linux-kernel@...r.kernel.org, linux-sh@...r.kernel.org,
Magnus Damm <magnus.damm@...il.com>,
Simon Horman <horms@...ge.net.au>,
Vinod Koul <vinod.koul@...el.com>,
Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Subject: Re: [PATCH v2 09/15] DMA: shdma: support referencing specific DMACs within a multiplexer in DT
Hi Guennadi,
On Monday 22 July 2013 08:34:19 Guennadi Liakhovetski wrote:
> On Mon, 22 Jul 2013, Laurent Pinchart wrote:
> > On Friday 19 July 2013 18:29:34 Guennadi Liakhovetski wrote:
> > > Currently shdma DT nodes have to be placed under a multiplexer node. DMA
> > > slave DT nodes then use that multiplexer's phandle in their "dmas"
> > > properties. However, sometimes it can be necessary to let DMA slaves
> > > only use a specific DMAC instance. In this case it would be logical to
> > > just use the respective phandle in that slave's "dmas" property. For
> > > this to work the referenced DMAC has to register a struct of_dma
> > > instance, which isn't presently done. Instead the driver currently only
> > > registers one struct of_dma for the multiplexer. This patch adds support
> > > for such configurations. To enable this option a "#dma-cells" property
> > > also must be added to the respective DMAC DT node.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@...il.com>
> > > ---
> > >
> > > Documentation/devicetree/bindings/dma/shdma.txt | 16 ++++++
> >
> > Patches that touch DT bindings must be CC'ed to devicetree@...r.kernel.org
> > (http://www.gossamer-threads.com/lists/linux/kernel/1750512).
>
> Yes, I saw that announcements, but if my calculations are correct, these
> patches went out before the MAINTAINERS patch above hit the mailing list,
> and as of this writing it's still not in next.
My point was merely to inform you of the change in case you had missed it :-)
> > > drivers/dma/sh/shdma.h | 7 +++
> > > drivers/dma/sh/shdmac.c | 66 ++++++++++++++++
> > > 3 files changed, 89 insertions(+), 0 deletions(-)
>
> [snip]
>
> > > diff --git a/drivers/dma/sh/shdmac.c b/drivers/dma/sh/shdmac.c
> > > index ae89261..0ddcddf 100644
> > > --- a/drivers/dma/sh/shdmac.c
> > > +++ b/drivers/dma/sh/shdmac.c
> > > @@ -22,6 +22,7 @@
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > #include <linux/of_device.h>
> > > +#include <linux/of_dma.h>
> > > #include <linux/slab.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/dmaengine.h>
> > > @@ -665,6 +666,63 @@ static const struct shdma_ops sh_dmae_shdma_ops = {
> > > .get_partial = sh_dmae_get_partial,
> > > };
> > >
> > > +static bool sh_dmae_chan_filter(struct dma_chan *chan, void *arg)
> > > +{
> > > + struct sh_dmae_filter_info *info = arg;
> > > + struct shdma_chan *schan = to_shdma_chan(chan);
> > > + int match = info->hw_req;
> > > +
> > > + if (match < 0)
> > > + /* No slave requested - arbitrary channel */
> > > + return true;
> > > +
> > > + dev_dbg(schan->dev, "%s(): trying %s for 0x%x\n", __func__,
> > > + info->of_node->full_name, match);
> > > +
> > > + if (schan->dev->of_node != info->of_node)
> > > + return false;
> > > +
> > > + return !sh_dmae_set_slave(schan, match, true);
> > > +}
> > > +
> > > +static struct dma_chan *sh_dmae_of_xlate(struct of_phandle_args
> > > *dma_spec,
> > > + struct of_dma *ofdma)
> > > +{
> > > + struct sh_dmae_filter_info *info = ofdma->of_dma_data;
> > > + u32 id = dma_spec->args[0];
> > > + dma_cap_mask_t mask;
> > > + struct dma_chan *chan;
> > > +
> > > + if (dma_spec->args_count != 1)
> > > + return NULL;
> > > +
> > > + dma_cap_zero(mask);
> > > + /* Only slave DMA channels can be allocated via DT */
> > > + dma_cap_set(DMA_SLAVE, mask);
> > > +
> > > + info->hw_req = id;
> > > + info->of_node = dma_spec->np;
> > > +
> > > + chan = dma_request_channel(mask, sh_dmae_chan_filter, info);
> > > + if (chan)
> > > + to_shdma_chan(chan)->hw_req = id;
> > > +
> > > + return chan;
> > > +}
> >
> > Would https://lkml.org/lkml/2013/3/25/250 help ?
>
> Don't think so. In my .xlate() method I also assign the .hw_req field,
If I'm not mistaken your hw_req field is equivalent to the chan_id field in
the above patch.
> and the filter is pretty different.
There's indeed an additional sh_dmae_set_slave() call in your filter, which
feels a bit out of place. I was just wondering whether it wouldn't be possible
to consolidate the code with the above patch.
> And in any case that patch is from March... And I don't see it in the
> mainline or in next, are there plans to re-push it?
I don't know, but I will need something similar soon, so I'll revive the
discussion.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists