| Message ID | 1449153192-9082-5-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote: > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > index 0675e268d577..46b305ea0d21 100644 > --- a/drivers/dma/edma.c > +++ b/drivers/dma/edma.c > @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev) > edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot); > } > > + if (info->slave_map) { > + ecc->dma_slave.filter_map.map = info->slave_map; > + ecc->dma_slave.filter_map.mapcnt = info->slavecnt; > + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn; > + } > + > Just a minor comment here: I think all three assignments can be done unconditionally. As I mentioned before, I'd also remove 'struct dma_filter' and put the three members in struct dma_device directly. In fact, the filter function can go with the other function pointers for consistency. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/03/2015 05:38 PM, Arnd Bergmann wrote: > On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote: >> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c >> index 0675e268d577..46b305ea0d21 100644 >> --- a/drivers/dma/edma.c >> +++ b/drivers/dma/edma.c >> @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev) >> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot); >> } >> >> + if (info->slave_map) { >> + ecc->dma_slave.filter_map.map = info->slave_map; >> + ecc->dma_slave.filter_map.mapcnt = info->slavecnt; >> + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn; >> + } >> + >> > > Just a minor comment here: I think all three assignments can be done > unconditionally. True. > As I mentioned before, I'd also remove 'struct dma_filter' > and put the three members in struct dma_device directly. In fact, the > filter function can go with the other function pointers for consistency. I just like to keep things in one place ;) I don't have strong stand on keeping the intermediate 'struct dma_filter' Let's hear from Vinod regarding to this
On 12/03/2015 05:46 PM, Peter Ujfalusi wrote: > On 12/03/2015 05:38 PM, Arnd Bergmann wrote: >> On Thursday 03 December 2015 16:33:12 Peter Ujfalusi wrote: >>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c >>> index 0675e268d577..46b305ea0d21 100644 >>> --- a/drivers/dma/edma.c >>> +++ b/drivers/dma/edma.c >>> @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev) >>> edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot); >>> } >>> >>> + if (info->slave_map) { >>> + ecc->dma_slave.filter_map.map = info->slave_map; >>> + ecc->dma_slave.filter_map.mapcnt = info->slavecnt; >>> + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn; >>> + } >>> + >>> >> >> Just a minor comment here: I think all three assignments can be done >> unconditionally. > > True. > >> As I mentioned before, I'd also remove 'struct dma_filter' >> and put the three members in struct dma_device directly. In fact, the >> filter function can go with the other function pointers for consistency. > > I just like to keep things in one place ;) > I don't have strong stand on keeping the intermediate 'struct dma_filter' > Let's hear from Vinod regarding to this One remaining design issue is on how and where to place the filter related variables/pointers: Keep it separated as it was in the RFC and v01 series: struct dma_slave_map { const char *devname; const char *slave; void *param; }; struct dma_filter { dma_filter_fn fn; int mapcnt; const struct dma_slave_map *map; }; struct dma_device { ... struct dma_filter filter; ... }; Or to have them under the dma_device directly: struct dma_device { ... int filter_mapcnt; const struct dma_slave_map *filter_map; ... dma_filter_fn filter_fn; ... }; Vinod: what is your preference for this? Thanks, Péter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 0675e268d577..46b305ea0d21 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -2297,6 +2297,12 @@ static int edma_probe(struct platform_device *pdev) edma_set_chmap(&ecc->slave_chans[i], ecc->dummy_slot); } + if (info->slave_map) { + ecc->dma_slave.filter_map.map = info->slave_map; + ecc->dma_slave.filter_map.mapcnt = info->slavecnt; + ecc->dma_slave.filter_map.filter_fn = edma_filter_fn; + } + ret = dma_async_device_register(&ecc->dma_slave); if (ret) { dev_err(dev, "slave ddev registration failed (%d)\n", ret); diff --git a/include/linux/platform_data/edma.h b/include/linux/platform_data/edma.h index e2878baeb90e..105700e62ea1 100644 --- a/include/linux/platform_data/edma.h +++ b/include/linux/platform_data/edma.h @@ -53,12 +53,16 @@ enum dma_event_q { #define EDMA_CTLR(i) ((i) >> 16) #define EDMA_CHAN_SLOT(i) ((i) & 0xffff) +#define EDMA_FILTER_PARAM(ctlr, chan) ((int[]) { EDMA_CTLR_CHAN(ctlr, chan) }) + struct edma_rsv_info { const s16 (*rsv_chans)[2]; const s16 (*rsv_slots)[2]; }; +struct dma_slave_map; + /* platform_data for EDMA driver */ struct edma_soc_info { /* @@ -76,6 +80,9 @@ struct edma_soc_info { s8 (*queue_priority_mapping)[2]; const s16 (*xbar_chans)[2]; + + const struct dma_slave_map *slave_map; + int slavecnt; }; #endif
Add support for providing device to filter_fn mapping so client drivers can switch to use the dma_request_chan() API. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- drivers/dma/edma.c | 6 ++++++ include/linux/platform_data/edma.h | 7 +++++++ 2 files changed, 13 insertions(+)