Message ID | 1499343623-5964-3-git-send-email-pierre-yves.mordret@st.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
On Thu, Jul 06, 2017 at 02:20:20PM +0200, Pierre-Yves MORDRET wrote: > +static int stm32_dmamux_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct device_node *dma_node; > + struct stm32_dmamux_data *stm32_dmamux; > + struct resource *res; > + void __iomem *iomem; > + int i, ret; > + > + if (!node) > + return -ENODEV; > + > + stm32_dmamux = devm_kzalloc(&pdev->dev, sizeof(*stm32_dmamux), > + GFP_KERNEL); > + if (!stm32_dmamux) > + return -ENOMEM; > + > + dma_node = of_parse_phandle(node, "dma-masters", 0); > + if (!dma_node) { > + dev_err(&pdev->dev, "Can't get DMA master node\n"); > + return -ENODEV; > + } > + > + if (device_property_read_u32(&pdev->dev, "dma-channels", > + &stm32_dmamux->dmamux_channels)) > + stm32_dmamux->dmamux_channels = STM32_DMAMUX_MAX_CHANNELS; > + > + if (device_property_read_u32(&pdev->dev, "dma-requests", > + &stm32_dmamux->dmamux_requests)) > + stm32_dmamux->dmamux_requests = STM32_DMAMUX_MAX_REQUESTS; I think defaults should be warned here too > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENODEV; > + > + iomem = devm_ioremap_resource(&pdev->dev, res); > + if (!iomem) > + return -ENOMEM; > + > + spin_lock_init(&stm32_dmamux->lock); > + > + stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(stm32_dmamux->clk)) { > + dev_info(&pdev->dev, "Missing controller clock\n"); Can you check for EPROBE_DEFER and print only for if that is not the error otherwise we end up sapmming with defered probe issues > + > +#ifndef __DMA_STM32_DMAMUX_H > +#define __DMA_STM32_DMAMUX_H > + > +#if defined(CONFIG_STM32_DMAMUX) > +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); Why do we need a custom API in this case?
DQoNCk9uIDA3LzIyLzIwMTcgMDg6NTEgQU0sIFZpbm9kIEtvdWwgd3JvdGU6DQo+IE9uIFRodSwg SnVsIDA2LCAyMDE3IGF0IDAyOjIwOjIwUE0gKzAyMDAsIFBpZXJyZS1ZdmVzIE1PUkRSRVQgd3Jv dGU6DQo+PiArc3RhdGljIGludCBzdG0zMl9kbWFtdXhfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2Rl dmljZSAqcGRldikNCj4+ICt7DQo+PiArCXN0cnVjdCBkZXZpY2Vfbm9kZSAqbm9kZSA9IHBkZXYt PmRldi5vZl9ub2RlOw0KPj4gKwlzdHJ1Y3QgZGV2aWNlX25vZGUgKmRtYV9ub2RlOw0KPj4gKwlz dHJ1Y3Qgc3RtMzJfZG1hbXV4X2RhdGEgKnN0bTMyX2RtYW11eDsNCj4+ICsJc3RydWN0IHJlc291 cmNlICpyZXM7DQo+PiArCXZvaWQgX19pb21lbSAqaW9tZW07DQo+PiArCWludCBpLCByZXQ7DQo+ PiArDQo+PiArCWlmICghbm9kZSkNCj4+ICsJCXJldHVybiAtRU5PREVWOw0KPj4gKw0KPj4gKwlz dG0zMl9kbWFtdXggPSBkZXZtX2t6YWxsb2MoJnBkZXYtPmRldiwgc2l6ZW9mKCpzdG0zMl9kbWFt dXgpLA0KPj4gKwkJCQkgICAgR0ZQX0tFUk5FTCk7DQo+PiArCWlmICghc3RtMzJfZG1hbXV4KQ0K Pj4gKwkJcmV0dXJuIC1FTk9NRU07DQo+PiArDQo+PiArCWRtYV9ub2RlID0gb2ZfcGFyc2VfcGhh bmRsZShub2RlLCAiZG1hLW1hc3RlcnMiLCAwKTsNCj4+ICsJaWYgKCFkbWFfbm9kZSkgew0KPj4g KwkJZGV2X2VycigmcGRldi0+ZGV2LCAiQ2FuJ3QgZ2V0IERNQSBtYXN0ZXIgbm9kZVxuIik7DQo+ PiArCQlyZXR1cm4gLUVOT0RFVjsNCj4+ICsJfQ0KPj4gKw0KPj4gKwlpZiAoZGV2aWNlX3Byb3Bl cnR5X3JlYWRfdTMyKCZwZGV2LT5kZXYsICJkbWEtY2hhbm5lbHMiLA0KPj4gKwkJCQkgICAgICZz dG0zMl9kbWFtdXgtPmRtYW11eF9jaGFubmVscykpDQo+PiArCQlzdG0zMl9kbWFtdXgtPmRtYW11 eF9jaGFubmVscyA9IFNUTTMyX0RNQU1VWF9NQVhfQ0hBTk5FTFM7DQo+PiArDQo+PiArCWlmIChk ZXZpY2VfcHJvcGVydHlfcmVhZF91MzIoJnBkZXYtPmRldiwgImRtYS1yZXF1ZXN0cyIsDQo+PiAr CQkJCSAgICAgJnN0bTMyX2RtYW11eC0+ZG1hbXV4X3JlcXVlc3RzKSkNCj4+ICsJCXN0bTMyX2Rt YW11eC0+ZG1hbXV4X3JlcXVlc3RzID0gU1RNMzJfRE1BTVVYX01BWF9SRVFVRVNUUzsNCj4gDQo+ IEkgdGhpbmsgZGVmYXVsdHMgc2hvdWxkIGJlIHdhcm5lZCBoZXJlIHRvbw0KPiANCg0Kb2sNCg0K Pj4gKw0KPj4gKwlyZXMgPSBwbGF0Zm9ybV9nZXRfcmVzb3VyY2UocGRldiwgSU9SRVNPVVJDRV9N RU0sIDApOw0KPj4gKwlpZiAoIXJlcykNCj4+ICsJCXJldHVybiAtRU5PREVWOw0KPj4gKw0KPj4g Kwlpb21lbSA9IGRldm1faW9yZW1hcF9yZXNvdXJjZSgmcGRldi0+ZGV2LCByZXMpOw0KPj4gKwlp ZiAoIWlvbWVtKQ0KPj4gKwkJcmV0dXJuIC1FTk9NRU07DQo+PiArDQo+PiArCXNwaW5fbG9ja19p bml0KCZzdG0zMl9kbWFtdXgtPmxvY2spOw0KPj4gKw0KPj4gKwlzdG0zMl9kbWFtdXgtPmNsayA9 IGRldm1fY2xrX2dldCgmcGRldi0+ZGV2LCBOVUxMKTsNCj4+ICsJaWYgKElTX0VSUihzdG0zMl9k bWFtdXgtPmNsaykpIHsNCj4+ICsJCWRldl9pbmZvKCZwZGV2LT5kZXYsICJNaXNzaW5nIGNvbnRy b2xsZXIgY2xvY2tcbiIpOw0KPiANCj4gQ2FuIHlvdSBjaGVjayBmb3IgRVBST0JFX0RFRkVSIGFu ZCBwcmludCBvbmx5IGZvciBpZiB0aGF0IGlzIG5vdCB0aGUgZXJyb3INCj4gb3RoZXJ3aXNlIHdl IGVuZCB1cCBzYXBtbWluZyB3aXRoIGRlZmVyZWQgcHJvYmUgaXNzdWVzDQo+IA0KDQpUaGlzIGlz IHdoYXQgeW91IG1lYW50ID8NCglpZiAoSVNfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKSAhPSBFUFJP QkVfREVGRVIpIHsNCgkJZGV2X2luZm8oJnBkZXYtPmRldiwgIk1pc3NpbmcgY29udHJvbGxlciBj bG9ja1xuIik7DQoJCXJldHVybiBQVFJfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKTsNCgl9DQoNCk9S DQoNCglpZiAoSVNfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKSkgew0KCQlpZiAoSVNfRVJSKHN0bTMy X2RtYW11eC0+Y2xrKSAhPSBFUFJPQkVfREVGRVIpDQoJCQlkZXZfaW5mbygmcGRldi0+ZGV2LCAi TWlzc2luZyBjb250cm9sbGVyIGNsb2NrXG4iKTsNCgkJcmV0dXJuIFBUUl9FUlIoc3RtMzJfZG1h bXV4LT5jbGspOw0KCX0NCg0KPj4gKw0KPj4gKyNpZm5kZWYgX19ETUFfU1RNMzJfRE1BTVVYX0gN Cj4+ICsjZGVmaW5lIF9fRE1BX1NUTTMyX0RNQU1VWF9IDQo+PiArDQo+PiArI2lmIGRlZmluZWQo Q09ORklHX1NUTTMyX0RNQU1VWCkNCj4+ICtpbnQgc3RtMzJfZG1hbXV4X3NldF9jb25maWcoc3Ry dWN0IGRldmljZSAqZGV2LCB2b2lkICpyb3V0ZV9kYXRhLCB1MzIgY2hhbl9pZCk7DQo+IA0KPiBX aHkgZG8gd2UgbmVlZCBhIGN1c3RvbSBBUEkgaW4gdGhpcyBjYXNlPw0KPiANCg0KVGhpcyBBUEkg aXMgY2FsbGVkIGJ5IERNQSB3aGVuIGEgc2xhdmUgaXMgcmVxdWVzdGVkIGJ5IGNsaWVudC4gRE1B IGNhbiB3b3JrDQp3aXRob3V0IERNQU1VWCB0aGlzIEFQSSBoYXMgYmVlbiBwdXQgaW4gcGxhY2Ug dG8gY29uZmlndXJlIERNQU1VWCB3aGV0aGVyIGNsaWVudA0KaXMgcmVxdWVzdGluZyBhIERNQU1V WCBDaGFubmVsIGluc3RlYWQgb2YgYSBETUEgb25lLg0KDQpUaGFua3MNClB5 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 24, 2017 at 01:55:10PM +0000, Pierre Yves MORDRET wrote: > >> + > >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> + if (!res) > >> + return -ENODEV; > >> + > >> + iomem = devm_ioremap_resource(&pdev->dev, res); > >> + if (!iomem) > >> + return -ENOMEM; > >> + > >> + spin_lock_init(&stm32_dmamux->lock); > >> + > >> + stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL); > >> + if (IS_ERR(stm32_dmamux->clk)) { > >> + dev_info(&pdev->dev, "Missing controller clock\n"); > > > > Can you check for EPROBE_DEFER and print only for if that is not the error > > otherwise we end up sapmming with defered probe issues > > > > This is what you meant ? > if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER) { > dev_info(&pdev->dev, "Missing controller clock\n"); > return PTR_ERR(stm32_dmamux->clk); > } > > OR > > if (IS_ERR(stm32_dmamux->clk)) { > if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER) > dev_info(&pdev->dev, "Missing controller clock\n"); > return PTR_ERR(stm32_dmamux->clk); > } This one please > > >> + > >> +#ifndef __DMA_STM32_DMAMUX_H > >> +#define __DMA_STM32_DMAMUX_H > >> + > >> +#if defined(CONFIG_STM32_DMAMUX) > >> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); > > > > Why do we need a custom API in this case? > > > > This API is called by DMA when a slave is requested by client. DMA can work > without DMAMUX this API has been put in place to configure DMAMUX whether client > is requesting a DMAMUX Channel instead of a DMA one. You mean the dmaengine driver right?
DQoNCk9uIDA3LzI2LzIwMTcgMDc6MjkgQU0sIFZpbm9kIEtvdWwgd3JvdGU6DQo+IE9uIE1vbiwg SnVsIDI0LCAyMDE3IGF0IDAxOjU1OjEwUE0gKzAwMDAsIFBpZXJyZSBZdmVzIE1PUkRSRVQgd3Jv dGU6DQo+IA0KPj4+PiArDQo+Pj4+ICsJcmVzID0gcGxhdGZvcm1fZ2V0X3Jlc291cmNlKHBkZXYs IElPUkVTT1VSQ0VfTUVNLCAwKTsNCj4+Pj4gKwlpZiAoIXJlcykNCj4+Pj4gKwkJcmV0dXJuIC1F Tk9ERVY7DQo+Pj4+ICsNCj4+Pj4gKwlpb21lbSA9IGRldm1faW9yZW1hcF9yZXNvdXJjZSgmcGRl di0+ZGV2LCByZXMpOw0KPj4+PiArCWlmICghaW9tZW0pDQo+Pj4+ICsJCXJldHVybiAtRU5PTUVN Ow0KPj4+PiArDQo+Pj4+ICsJc3Bpbl9sb2NrX2luaXQoJnN0bTMyX2RtYW11eC0+bG9jayk7DQo+ Pj4+ICsNCj4+Pj4gKwlzdG0zMl9kbWFtdXgtPmNsayA9IGRldm1fY2xrX2dldCgmcGRldi0+ZGV2 LCBOVUxMKTsNCj4+Pj4gKwlpZiAoSVNfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKSkgew0KPj4+PiAr CQlkZXZfaW5mbygmcGRldi0+ZGV2LCAiTWlzc2luZyBjb250cm9sbGVyIGNsb2NrXG4iKTsNCj4+ Pg0KPj4+IENhbiB5b3UgY2hlY2sgZm9yIEVQUk9CRV9ERUZFUiBhbmQgcHJpbnQgb25seSBmb3Ig aWYgdGhhdCBpcyBub3QgdGhlIGVycm9yDQo+Pj4gb3RoZXJ3aXNlIHdlIGVuZCB1cCBzYXBtbWlu ZyB3aXRoIGRlZmVyZWQgcHJvYmUgaXNzdWVzDQo+Pj4NCj4+DQo+PiBUaGlzIGlzIHdoYXQgeW91 IG1lYW50ID8NCj4+IAlpZiAoSVNfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKSAhPSBFUFJPQkVfREVG RVIpIHsNCj4+IAkJZGV2X2luZm8oJnBkZXYtPmRldiwgIk1pc3NpbmcgY29udHJvbGxlciBjbG9j a1xuIik7DQo+PiAJCXJldHVybiBQVFJfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKTsNCj4+IAl9DQo+ Pg0KPj4gT1INCj4+DQo+PiAJaWYgKElTX0VSUihzdG0zMl9kbWFtdXgtPmNsaykpIHsNCj4+IAkJ aWYgKElTX0VSUihzdG0zMl9kbWFtdXgtPmNsaykgIT0gRVBST0JFX0RFRkVSKQ0KPj4gCQkJZGV2 X2luZm8oJnBkZXYtPmRldiwgIk1pc3NpbmcgY29udHJvbGxlciBjbG9ja1xuIik7DQo+PiAJCXJl dHVybiBQVFJfRVJSKHN0bTMyX2RtYW11eC0+Y2xrKTsNCj4+IAl9DQo+IA0KPiBUaGlzIG9uZSBw bGVhc2UNCj4gDQoNCm9rDQoNCj4+DQo+Pj4+ICsNCj4+Pj4gKyNpZm5kZWYgX19ETUFfU1RNMzJf RE1BTVVYX0gNCj4+Pj4gKyNkZWZpbmUgX19ETUFfU1RNMzJfRE1BTVVYX0gNCj4+Pj4gKw0KPj4+ PiArI2lmIGRlZmluZWQoQ09ORklHX1NUTTMyX0RNQU1VWCkNCj4+Pj4gK2ludCBzdG0zMl9kbWFt dXhfc2V0X2NvbmZpZyhzdHJ1Y3QgZGV2aWNlICpkZXYsIHZvaWQgKnJvdXRlX2RhdGEsIHUzMiBj aGFuX2lkKTsNCj4+Pg0KPj4+IFdoeSBkbyB3ZSBuZWVkIGEgY3VzdG9tIEFQSSBpbiB0aGlzIGNh c2U/DQo+Pj4NCj4+DQo+PiBUaGlzIEFQSSBpcyBjYWxsZWQgYnkgRE1BIHdoZW4gYSBzbGF2ZSBp cyByZXF1ZXN0ZWQgYnkgY2xpZW50LiBETUEgY2FuIHdvcmsNCj4+IHdpdGhvdXQgRE1BTVVYIHRo aXMgQVBJIGhhcyBiZWVuIHB1dCBpbiBwbGFjZSB0byBjb25maWd1cmUgRE1BTVVYIHdoZXRoZXIg Y2xpZW50DQo+PiBpcyByZXF1ZXN0aW5nIGEgRE1BTVVYIENoYW5uZWwgaW5zdGVhZCBvZiBhIERN QSBvbmUuDQo+IA0KPiBZb3UgbWVhbiB0aGUgZG1hZW5naW5lIGRyaXZlciByaWdodD8NCj4gDQoN Clllcy4gVGhlIEFQSSBpcyBtYWlubHkgY2FsbGVkIGJ5ICJkZXZpY2VfY29uZmlnIiB0aHJvdWdo IG91dCBTVE0zMiBETUEgRHJpdmVyDQp3aGVuIGEgcm91dGVyIGlzIGluIHBsYWNlIGZvciBjbGll bnQuDQpQbGVhc2UgcmVmZXIgdG8gUGF0Y2ggNC81IG9uIHRoaXMgc2V0Lg0KDQpUaGFua3MNClB5 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote: > >>>> + > >>>> +#ifndef __DMA_STM32_DMAMUX_H > >>>> +#define __DMA_STM32_DMAMUX_H > >>>> + > >>>> +#if defined(CONFIG_STM32_DMAMUX) > >>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); > >>> > >>> Why do we need a custom API in this case? > >>> > >> > >> This API is called by DMA when a slave is requested by client. DMA can work > >> without DMAMUX this API has been put in place to configure DMAMUX whether client > >> is requesting a DMAMUX Channel instead of a DMA one. > > > > You mean the dmaengine driver right? > > > > Yes. The API is mainly called by "device_config" through out STM32 DMA Driver > when a router is in place for client. > Please refer to Patch 4/5 on this set. Okay am thinking on why this can't be generic..? An optional router config callback?
DQoNCk9uIDA3LzMxLzIwMTcgMDI6MzEgUE0sIFZpbm9kIEtvdWwgd3JvdGU6DQo+IE9uIFdlZCwg SnVsIDI2LCAyMDE3IGF0IDA3OjM4OjAyQU0gKzAwMDAsIFBpZXJyZSBZdmVzIE1PUkRSRVQgd3Jv dGU6DQo+Pj4+Pj4gKw0KPj4+Pj4+ICsjaWZuZGVmIF9fRE1BX1NUTTMyX0RNQU1VWF9IDQo+Pj4+ Pj4gKyNkZWZpbmUgX19ETUFfU1RNMzJfRE1BTVVYX0gNCj4+Pj4+PiArDQo+Pj4+Pj4gKyNpZiBk ZWZpbmVkKENPTkZJR19TVE0zMl9ETUFNVVgpDQo+Pj4+Pj4gK2ludCBzdG0zMl9kbWFtdXhfc2V0 X2NvbmZpZyhzdHJ1Y3QgZGV2aWNlICpkZXYsIHZvaWQgKnJvdXRlX2RhdGEsIHUzMiBjaGFuX2lk KTsNCj4+Pj4+DQo+Pj4+PiBXaHkgZG8gd2UgbmVlZCBhIGN1c3RvbSBBUEkgaW4gdGhpcyBjYXNl Pw0KPj4+Pj4NCj4+Pj4NCj4+Pj4gVGhpcyBBUEkgaXMgY2FsbGVkIGJ5IERNQSB3aGVuIGEgc2xh dmUgaXMgcmVxdWVzdGVkIGJ5IGNsaWVudC4gRE1BIGNhbiB3b3JrDQo+Pj4+IHdpdGhvdXQgRE1B TVVYIHRoaXMgQVBJIGhhcyBiZWVuIHB1dCBpbiBwbGFjZSB0byBjb25maWd1cmUgRE1BTVVYIHdo ZXRoZXIgY2xpZW50DQo+Pj4+IGlzIHJlcXVlc3RpbmcgYSBETUFNVVggQ2hhbm5lbCBpbnN0ZWFk IG9mIGEgRE1BIG9uZS4NCj4+Pg0KPj4+IFlvdSBtZWFuIHRoZSBkbWFlbmdpbmUgZHJpdmVyIHJp Z2h0Pw0KPj4+DQo+Pg0KPj4gWWVzLiBUaGUgQVBJIGlzIG1haW5seSBjYWxsZWQgYnkgImRldmlj ZV9jb25maWciIHRocm91Z2ggb3V0IFNUTTMyIERNQSBEcml2ZXINCj4+IHdoZW4gYSByb3V0ZXIg aXMgaW4gcGxhY2UgZm9yIGNsaWVudC4NCj4+IFBsZWFzZSByZWZlciB0byBQYXRjaCA0LzUgb24g dGhpcyBzZXQuDQo+IA0KPiBPa2F5IGFtIHRoaW5raW5nIG9uIHdoeSB0aGlzIGNhbid0IGJlIGdl bmVyaWMuLj8gQW4gb3B0aW9uYWwgcm91dGVyIGNvbmZpZw0KPiBjYWxsYmFjaz8NCj4gDQoNCkkg d291bGQgaGF2ZSBsaWtlZCB0byBhbnN3ZXIgdGhlcmUgaXMgYSBjYWxsYmFjayB3aXRoaW4gZW5n aW5lIGJ1dCB1bmZvcnR1bmF0ZWx5DQpJIGRpZG4ndCBmaWd1cmUgb3V0IG9uZSB3aGVuIEkgZGlk IG15IHJvdXRlcidzIGRldmVsb3BtZW50LiBJJ3ZlIGxvb2tlZCBvbmNlDQptb3JlIGJ1dCBhZ2Fp biBJIGNhbid0IGZpbmQgaG93IHRvIG1hcCBjaGFuSUQgYW5kIHJlcXVlc3QgbGluZSB3aXRob3V0 IGN1c3RvbSBBUEku -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote: > > > On 07/31/2017 02:31 PM, Vinod Koul wrote: > > On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote: > >>>>>> + > >>>>>> +#ifndef __DMA_STM32_DMAMUX_H > >>>>>> +#define __DMA_STM32_DMAMUX_H > >>>>>> + > >>>>>> +#if defined(CONFIG_STM32_DMAMUX) > >>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); > >>>>> > >>>>> Why do we need a custom API in this case? > >>>>> > >>>> > >>>> This API is called by DMA when a slave is requested by client. DMA can work > >>>> without DMAMUX this API has been put in place to configure DMAMUX whether client > >>>> is requesting a DMAMUX Channel instead of a DMA one. > >>> > >>> You mean the dmaengine driver right? > >>> > >> > >> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver > >> when a router is in place for client. > >> Please refer to Patch 4/5 on this set. > > > > Okay am thinking on why this can't be generic..? An optional router config > > callback? > > > > I would have liked to answer there is a callback within engine but unfortunately > I didn't figure out one when I did my router's development. I've looked once > more but again I can't find how to map chanID and request line without custom API. Yes there is no callback for routers but we can add a generic callback here to be used. I added Peter for his comments, isn't that something they need too?
On 2017-08-02 07:55, Vinod Koul wrote: > On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote: >> >> >> On 07/31/2017 02:31 PM, Vinod Koul wrote: >>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote: >>>>>>>> + >>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H >>>>>>>> +#define __DMA_STM32_DMAMUX_H >>>>>>>> + >>>>>>>> +#if defined(CONFIG_STM32_DMAMUX) >>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); >>>>>>> >>>>>>> Why do we need a custom API in this case? >>>>>>> >>>>>> >>>>>> This API is called by DMA when a slave is requested by client. DMA can work >>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client >>>>>> is requesting a DMAMUX Channel instead of a DMA one. >>>>> >>>>> You mean the dmaengine driver right? >>>>> >>>> >>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver >>>> when a router is in place for client. >>>> Please refer to Patch 4/5 on this set. >>> >>> Okay am thinking on why this can't be generic..? An optional router config >>> callback? >>> >> >> I would have liked to answer there is a callback within engine but unfortunately >> I didn't figure out one when I did my router's development. I've looked once >> more but again I can't find how to map chanID and request line without custom API. > > Yes there is no callback for routers but we can add a generic callback > here to be used. I added Peter for his comments, isn't that something they > need too? The event router via of_dma_router_register() should be capable of handling different type of muxers, like the ti-dma-crossbar.c is doing for two different type of event crossbars. Basically with the of_dma_route_allocate you craft a dma_spec which can be understood by the dma-master pointed form the router's node. You do the configuration of the mux in this function, craft the dma_spec and that's it. In DT the peripherals are using the router's node for DMA binding and everything is transparent for them. Note: The use of am335x xbar in the ti-dma-crossbar is optional and only needed when we need to have different event than the default for a specific dma request line. If you normally use the DMA like this: dmas = <&edma 129 1>, <&ddma_xbar 128 1>; dma-names = "tx", "rx"; If you have DMA event router/mux, then depending on how it works you might have different number of parameters. In my case the DRA7 crossbar does not need extra parameter, so to get it in use: dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>; dma-names = "tx", "rx"; The router driver will rewrite the dma_spec and replace the 129/128 and pass something different to the dma-master (dynamic event mapping). On am335x we have different xbar type so there: dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>; Out from this the router driver will create a spec equivalent to dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>; But it will change the xbar that DMA request 12/13 will not have the default event routed to. I believe that the dma_router infra we have in dmaengine can cover most if not all use cases... - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/02/2017 11:19 AM, Peter Ujfalusi wrote: > > > On 2017-08-02 07:55, Vinod Koul wrote: >> On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote: >>> >>> >>> On 07/31/2017 02:31 PM, Vinod Koul wrote: >>>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote: >>>>>>>>> + >>>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H >>>>>>>>> +#define __DMA_STM32_DMAMUX_H >>>>>>>>> + >>>>>>>>> +#if defined(CONFIG_STM32_DMAMUX) >>>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); >>>>>>>> >>>>>>>> Why do we need a custom API in this case? >>>>>>>> >>>>>>> >>>>>>> This API is called by DMA when a slave is requested by client. DMA can work >>>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client >>>>>>> is requesting a DMAMUX Channel instead of a DMA one. >>>>>> >>>>>> You mean the dmaengine driver right? >>>>>> >>>>> >>>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver >>>>> when a router is in place for client. >>>>> Please refer to Patch 4/5 on this set. >>>> >>>> Okay am thinking on why this can't be generic..? An optional router config >>>> callback? >>>> >>> >>> I would have liked to answer there is a callback within engine but unfortunately >>> I didn't figure out one when I did my router's development. I've looked once >>> more but again I can't find how to map chanID and request line without custom API. >> >> Yes there is no callback for routers but we can add a generic callback >> here to be used. I added Peter for his comments, isn't that something they >> need too? > > The event router via of_dma_router_register() should be capable of > handling different type of muxers, like the ti-dma-crossbar.c is doing > for two different type of event crossbars. > > Basically with the of_dma_route_allocate you craft a dma_spec which can > be understood by the dma-master pointed form the router's node. > You do the configuration of the mux in this function, craft the dma_spec > and that's it. In DT the peripherals are using the router's node for DMA > binding and everything is transparent for them. > > Note: The use of am335x xbar in the ti-dma-crossbar is optional and only > needed when we need to have different event than the default for a > specific dma request line. > > If you normally use the DMA like this: > dmas = <&edma 129 1>, <&ddma_xbar 128 1>; > dma-names = "tx", "rx"; > > If you have DMA event router/mux, then depending on how it works you > might have different number of parameters. In my case the DRA7 crossbar > does not need extra parameter, so to get it in use: > dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>; > dma-names = "tx", "rx"; > > The router driver will rewrite the dma_spec and replace the 129/128 and > pass something different to the dma-master (dynamic event mapping). > > On am335x we have different xbar type so there: > > dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>; > > Out from this the router driver will create a spec equivalent to > dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>; > > But it will change the xbar that DMA request 12/13 will not have the > default event routed to. > > I believe that the dma_router infra we have in dmaengine can cover most > if not all use cases... > > - Péter > Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a ChannelId and request line is part of the binding. Using DMAMUx now the request line is coming from dma_spec forwards to dma-master as well explained by Peter. However ChannelID is now given by dma_get_any_slave_channel instead of bindings. DMAMUX driver has to be aware of this ID to route request line to out DMA channel. This channel id information is carried on until DMAMUX through dmaengine_slave_config with a custom API. Hope it clarifies the need.
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-08-02 16:11, Pierre Yves MORDRET wrote: > Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a > ChannelId and request line is part of the binding. In our case the am335x's eDMA can work with or without the router, we only use the router node if we need none default event for a given DMA request line. > Using DMAMUx now the request > line is coming from dma_spec forwards to dma-master as well explained by Peter. > However ChannelID is now given by dma_get_any_slave_channel instead of bindings. > DMAMUX driver has to be aware of this ID to route request line to out DMA > channel. This channel id information is carried on until DMAMUX through > dmaengine_slave_config with a custom API. > Hope it clarifies the need. I see, this is not much different then what we face with our dra7 devices. In theory we could use direct DMA binding to the DMA controller itself, but some requests would not be reachable, so we always use the router's node for DMA on dra7 family. Basically the router would manage the ChannelID and create 'st,stm32-dma' compatible dma_spec (the four parameters). Afaik you could have 3 parameters for the router and create a four parameter dma_spec, where the ChannelID is dynamically allocated. But you need to convert all peripherals to use the router's node for the DMA. - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/02/2017 04:09 PM, Peter Ujfalusi wrote: > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 2017-08-02 16:11, Pierre Yves MORDRET wrote: >> Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a >> ChannelId and request line is part of the binding. > > In our case the am335x's eDMA can work with or without the router, we > only use the router node if we need none default event for a given DMA > request line. > >> Using DMAMUx now the request >> line is coming from dma_spec forwards to dma-master as well explained by Peter. >> However ChannelID is now given by dma_get_any_slave_channel instead of bindings. >> DMAMUX driver has to be aware of this ID to route request line to out DMA >> channel. This channel id information is carried on until DMAMUX through >> dmaengine_slave_config with a custom API. >> Hope it clarifies the need. > > I see, this is not much different then what we face with our dra7 > devices. In theory we could use direct DMA binding to the DMA controller > itself, but some requests would not be reachable, so we always use the > router's node for DMA on dra7 family. > > Basically the router would manage the ChannelID and create > 'st,stm32-dma' compatible dma_spec (the four parameters). > Afaik you could have 3 parameters for the router and create a four > parameter dma_spec, where the ChannelID is dynamically allocated. Correct router needs 3 parameters and among those 2 are forwarded though out dma_spec. But when you say "ChannelID is dynamically allocated" you mean dma_get_any_slave_channel ? If yes I can use the already existing bindings to carry the channelID to DMA. No changes need to peripheral... > But you need to convert all peripherals to use the router's node for the > DMA. > > - Péter > Py -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
our mail server started to mangle outgoing mails, sorry for that, we are trying to resolve that... Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-08-02 17:28, Pierre Yves MORDRET wrote: > Correct router needs 3 parameters and among those 2 are forwarded though out > dma_spec. But when you say "ChannelID is dynamically allocated" you mean > dma_get_any_slave_channel ? If yes I can use the already existing bindings to > carry the channelID to DMA. No changes need to peripheral... What I actually mean is that you should not need to modify the DMA driver at all. According to stm32-dma.txt: #dma-cells = <4>; 1. channelID 2. request line number 3. - 4. some parameters I believe if you don't have the event router (directly using the DMA node) you always need to provide these, right? If I'm not mistaken, when you use the event router you want to omit the ChannelID and get a random channel via dma_get_any_slave_channel in the DMA driver and feed back the channelID to the event router driver? I believe the reason for this is that you want to keep mixed binding use, to have direct DMA bindings and bindings via event router. Now what happens if you have direct binding: device1 { dmas = <&dma2 1 4 0x10400 0x3>; }; and via event router: device2 { dmas = <&dma_router 10 0x10400 0x3>, <&dma_router 11 0x10400 0x3>; }; device2 probes first, will get channelID 0 and 1 via dma_get_any_slave_channel. When device1 tries to probe, the channelID 1 is already taken.. You need to convert all peripherals to use the event router to avoid such a races. I have done the same for the dra7. Add the event router driver, add the event router node and convert existing users to use that when adding new devices, use the event router node. The event router's binding would have 3 parameters, it manages the available channelIDs, like the ti-dma-crossbar does for dra7. At allocate time it would pick an unused channelID and craft the dma-spec with the four parameters as documented. The main DMA driver will not need any modification as everything will be taken care of by the event router. The only gotcha is with memcpy type of transfers as they might also need unique channelID, but not requested via the slave binding. For that I have added properties to the event router to mask out certain channels (and I needed to do the same for the eDMA, but it is unrelated to the router itself). - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2017 08:42 AM, Peter Ujfalusi wrote: > our mail server started to mangle outgoing mails, sorry for that, we are > trying to resolve that... No problem ;) > > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 2017-08-02 17:28, Pierre Yves MORDRET wrote: >> Correct router needs 3 parameters and among those 2 are forwarded though out >> dma_spec. But when you say "ChannelID is dynamically allocated" you mean >> dma_get_any_slave_channel ? If yes I can use the already existing bindings to >> carry the channelID to DMA. No changes need to peripheral... > > What I actually mean is that you should not need to modify the DMA > driver at all. > According to stm32-dma.txt: > #dma-cells = <4>; > 1. channelID > 2. request line number > 3. - 4. some parameters > > I believe if you don't have the event router (directly using the DMA > node) you always need to provide these, right? Correct > If I'm not mistaken, when you use the event router you want to omit the > ChannelID and get a random channel via dma_get_any_slave_channel in the > DMA driver and feed back the channelID to the event router driver? Again correct. > I believe the reason for this is that you want to keep mixed binding > use, to have direct DMA bindings and bindings via event router. > Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is more for backward compatibility with SoC which doesn't have a DMAMUX. > Now what happens if you have direct binding: > device1 { > dmas = <&dma2 1 4 0x10400 0x3>; > }; > > and via event router: > device2 { > dmas = <&dma_router 10 0x10400 0x3>, > <&dma_router 11 0x10400 0x3>; > }; > > device2 probes first, will get channelID 0 and 1 via > dma_get_any_slave_channel. > > When device1 tries to probe, the channelID 1 is already taken.. Yes this is a flaw if we mix up bindings. > > You need to convert all peripherals to use the event router to avoid > such a races. I have done the same for the dra7. > Add the event router driver, > add the event router node and convert existing users to use that > when adding new devices, use the event router node. > > The event router's binding would have 3 parameters, it manages the > available channelIDs, like the ti-dma-crossbar does for dra7. At > allocate time it would pick an unused channelID and craft the dma-spec > with the four parameters as documented. > The main DMA driver will not need any modification as everything will be > taken care of by the event router. > I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver has been well inspired from ti-dma-crossbar. Nonetheless I understand what you meant. The channelID doesn't come from the dmaengine and a piece a code is devised to allocate those. I could copy/paste such code in my side but I do believe this would be better if such information would come from dmaengine instead : this is what I did but a link/callback is missing to craft this info until DMA. ChannelID is computed in two places in dmaemgine and in your driver. Moreover any router is going to develop its own channelID allocator, info which normally comes from dmaengine. Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the custom API. This is s rather big change to evaluate in my side though. However it seems to me such info should have come from dmaengine and not from driver. Let me know your thought about this > The only gotcha is with memcpy type of transfers as they might also need > unique channelID, but not requested via the slave binding. For that I > have added properties to the event router to mask out certain channels > (and I needed to do the same for the eDMA, but it is unrelated to the > router itself). > > > - Péter > Py -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-08-03 12:00, Pierre Yves MORDRET wrote: >> What I actually mean is that you should not need to modify the DMA >> driver at all. >> According to stm32-dma.txt: >> #dma-cells = <4>; >> 1. channelID >> 2. request line number >> 3. - 4. some parameters >> >> I believe if you don't have the event router (directly using the DMA >> node) you always need to provide these, right? > > Correct > >> If I'm not mistaken, when you use the event router you want to omit the >> ChannelID and get a random channel via dma_get_any_slave_channel in the >> DMA driver and feed back the channelID to the event router driver? > > Again correct. > >> I believe the reason for this is that you want to keep mixed binding >> use, to have direct DMA bindings and bindings via event router. >> > > Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is > more for backward compatibility with SoC which doesn't have a DMAMUX. > >> Now what happens if you have direct binding: >> device1 { >> dmas = <&dma2 1 4 0x10400 0x3>; >> }; >> >> and via event router: >> device2 { >> dmas = <&dma_router 10 0x10400 0x3>, >> <&dma_router 11 0x10400 0x3>; >> }; >> >> device2 probes first, will get channelID 0 and 1 via >> dma_get_any_slave_channel. >> >> When device1 tries to probe, the channelID 1 is already taken.. > > Yes this is a flaw if we mix up bindings. > >> >> You need to convert all peripherals to use the event router to avoid >> such a races. I have done the same for the dra7. >> Add the event router driver, >> add the event router node and convert existing users to use that >> when adding new devices, use the event router node. >> >> The event router's binding would have 3 parameters, it manages the >> available channelIDs, like the ti-dma-crossbar does for dra7. At >> allocate time it would pick an unused channelID and craft the dma-spec >> with the four parameters as documented. >> The main DMA driver will not need any modification as everything will be >> taken care of by the event router. >> > > I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver > has been well inspired from ti-dma-crossbar. > Nonetheless I understand what you meant. The channelID doesn't come from the > dmaengine and a piece a code is devised to allocate those. I could copy/paste > such code in my side but I do believe this would be better if such information > would come from dmaengine instead : this is what I did but a link/callback is > missing to craft this info until DMA. ChannelID is computed in two places in > dmaemgine and in your driver. Moreover any router is going to develop its own > channelID allocator, info which normally comes from dmaengine. yes and no. In my case on dr7 we have DMA request crossbar to support more events than either eDMA or sDMA could possible handle. eDMA and sDMA works in different ways, but the same event router driver facilitates them fine. In sDMA any channel can service any DMA requests, while in eDMA the channel number and the event numbers are matched (ch10 can service only event10). Our event router driver's task is to map the incoming event number to an outgoing event number, which is then going to be used by the DMA driver as incoming event. So in the crossbar we anyways need to pick an event from the available list of unused ones. The DMA drivers (eDMA or sDMA) would just think that the request comes via normal binding and does what it normally does on SoC where we don't have the crossbar (OMAPs for sDMA, daVinci, am33/am43, k2g, etc for eDMA). I'm not sure what your DMAMUX muxes, is it the channels or the events, but it only need to manage the needed, moving parts. > Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the > custom API. This is s rather big change to evaluate in my side though. > However it seems to me such info should have come from dmaengine and not from > driver. > Let me know your thought about this > >> The only gotcha is with memcpy type of transfers as they might also need >> unique channelID, but not requested via the slave binding. For that I >> have added properties to the event router to mask out certain channels >> (and I needed to do the same for the eDMA, but it is unrelated to the >> router itself). >> >> >> - Péter >> > > Py > - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2017 11:48 AM, Peter Ujfalusi wrote: > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 2017-08-03 12:00, Pierre Yves MORDRET wrote: >>> What I actually mean is that you should not need to modify the DMA >>> driver at all. >>> According to stm32-dma.txt: >>> #dma-cells = <4>; >>> 1. channelID >>> 2. request line number >>> 3. - 4. some parameters >>> >>> I believe if you don't have the event router (directly using the DMA >>> node) you always need to provide these, right? >> >> Correct >> >>> If I'm not mistaken, when you use the event router you want to omit the >>> ChannelID and get a random channel via dma_get_any_slave_channel in the >>> DMA driver and feed back the channelID to the event router driver? >> >> Again correct. >> >>> I believe the reason for this is that you want to keep mixed binding >>> use, to have direct DMA bindings and bindings via event router. >>> >> >> Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is >> more for backward compatibility with SoC which doesn't have a DMAMUX. >> >>> Now what happens if you have direct binding: >>> device1 { >>> dmas = <&dma2 1 4 0x10400 0x3>; >>> }; >>> >>> and via event router: >>> device2 { >>> dmas = <&dma_router 10 0x10400 0x3>, >>> <&dma_router 11 0x10400 0x3>; >>> }; >>> >>> device2 probes first, will get channelID 0 and 1 via >>> dma_get_any_slave_channel. >>> >>> When device1 tries to probe, the channelID 1 is already taken.. >> >> Yes this is a flaw if we mix up bindings. >> >>> >>> You need to convert all peripherals to use the event router to avoid >>> such a races. I have done the same for the dra7. >>> Add the event router driver, >>> add the event router node and convert existing users to use that >>> when adding new devices, use the event router node. >>> >>> The event router's binding would have 3 parameters, it manages the >>> available channelIDs, like the ti-dma-crossbar does for dra7. At >>> allocate time it would pick an unused channelID and craft the dma-spec >>> with the four parameters as documented. >>> The main DMA driver will not need any modification as everything will be >>> taken care of by the event router. >>> >> >> I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver >> has been well inspired from ti-dma-crossbar. >> Nonetheless I understand what you meant. The channelID doesn't come from the >> dmaengine and a piece a code is devised to allocate those. I could copy/paste >> such code in my side but I do believe this would be better if such information >> would come from dmaengine instead : this is what I did but a link/callback is >> missing to craft this info until DMA. ChannelID is computed in two places in >> dmaemgine and in your driver. Moreover any router is going to develop its own >> channelID allocator, info which normally comes from dmaengine. > > yes and no. > In my case on dr7 we have DMA request crossbar to support more events > than either eDMA or sDMA could possible handle. eDMA and sDMA works in > different ways, but the same event router driver facilitates them fine. > In sDMA any channel can service any DMA requests, while in eDMA the > channel number and the event numbers are matched (ch10 can service only > event10). > > Our event router driver's task is to map the incoming event number to an > outgoing event number, which is then going to be used by the DMA driver > as incoming event. So in the crossbar we anyways need to pick an event > from the available list of unused ones. The DMA drivers (eDMA or sDMA) > would just think that the request comes via normal binding and does what > it normally does on SoC where we don't have the crossbar (OMAPs for > sDMA, daVinci, am33/am43, k2g, etc for eDMA). > > I'm not sure what your DMAMUX muxes, is it the channels or the events, > but it only need to manage the needed, moving parts. > Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the second DMA. Request line numbering is fixed (a peripheral DMA request is assigned to one MUX input) and but can be routed randomly onto the any 16 channels. We use chanID to mux input on event. chanID given by dma_get_any_slave_channe is enough in our case. Py >> Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the >> custom API. This is s rather big change to evaluate in my side though. >> However it seems to me such info should have come from dmaengine and not from >> driver. >> Let me know your thought about this >> >>> The only gotcha is with memcpy type of transfers as they might also need >>> unique channelID, but not requested via the slave binding. For that I >>> have added properties to the event router to mask out certain channels >>> (and I needed to do the same for the eDMA, but it is unrelated to the >>> router itself). >>> >>> >>> - Péter >>> >> >> Py >> > > - Péter > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/04/2017 03:50 PM, Pierre Yves MORDRET wrote: > Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned > though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the > second DMA. Request line numbering is fixed (a peripheral DMA request is > assigned to one MUX input) and but can be routed randomly onto the any 16 > channels. We use chanID to mux input on event. > chanID given by dma_get_any_slave_channe is enough in our case. I would think that if you have in the router node: dma-masters = <&dma1>, <&dma2>; and request a DMA via the router: dmas = <&dma_router req_in param1 param2>; then the router driver would decide which dma-master it is going to assign the given request line and craft the dma-spec based on this decision. This requires no callback to the router from the DMA master driver at all. The idea of the dma event router is to be transparent for the DMA clients (peripherals needing DMA channel) and for the DMA drivers as well. Neither should know that the events are muxed as it does not really matter for them.
On 08/04/2017 04:21 PM, Peter Ujfalusi wrote: > On 08/04/2017 03:50 PM, Pierre Yves MORDRET wrote: >> Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned >> though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the >> second DMA. Request line numbering is fixed (a peripheral DMA request is >> assigned to one MUX input) and but can be routed randomly onto the any 16 >> channels. We use chanID to mux input on event. >> chanID given by dma_get_any_slave_channe is enough in our case. > > I would think that if you have in the router node: > dma-masters = <&dma1>, <&dma2>; > > and request a DMA via the router: > dmas = <&dma_router req_in param1 param2>; > > then the router driver would decide which dma-master it is going to assign the > given request line and craft the dma-spec based on this decision. This > requires no callback to the router from the DMA master driver at all. > > The idea of the dma event router is to be transparent for the DMA clients > (peripherals needing DMA channel) and for the DMA drivers as well. Neither > should know that the events are muxed as it does not really matter for them. > OK. I will redesign my driver to take into account this idea. I believe I should get rid of my custom API in DMA for channelID as well. Please confirm. Not very clear for me whether I can keep it or not. Regards PY -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017-08-21 12:34, Pierre Yves MORDRET wrote: > OK. I will redesign my driver to take into account this idea. > > I believe I should get rid of my custom API in DMA for channelID as well. Please > confirm. Not very clear for me whether I can keep it or not. Yes, you should be able to get rid of any custom API. The DMA event router should be 'invisible' to both the DMA driver itself and for the DMA users as well. If you end up still needing custom API (which I doubt) then it is better to extend the core support to cover that in a generic way since it is likely that other might have similar needs. You need to identify what you need to manage with the DMA router, again in my cases: am335x/am437x: The DMA channel is fixed, but the DMA event to be handled by the channel (DMA event muxer) is selected by the router. dra7x: The DMA event is the fixed part and I needed to translate that to local eDMA/sDMA events and in eDMA I needed to pick a channel based on the translated event. The same router code works for eDMA and sDMA in dra7, while the two DMA engines are different in their internal works. At the end of the day I only needed to plug the DMA event routers and there is no change in the DMA drivers at all. I'm happy to answer any questions you might have, if I can. - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/24/2017 07:47 AM, Peter Ujfalusi wrote: > > > On 2017-08-21 12:34, Pierre Yves MORDRET wrote: >> OK. I will redesign my driver to take into account this idea. >> >> I believe I should get rid of my custom API in DMA for channelID as well. Please >> confirm. Not very clear for me whether I can keep it or not. > > Yes, you should be able to get rid of any custom API. > The DMA event router should be 'invisible' to both the DMA driver itself > and for the DMA users as well. If you end up still needing custom API > (which I doubt) then it is better to extend the core support to cover > that in a generic way since it is likely that other might have similar > needs. > Will see that later on :) > You need to identify what you need to manage with the DMA router, again > in my cases: > am335x/am437x: The DMA channel is fixed, but the DMA event to be handled > by the channel (DMA event muxer) is selected by the router. > dra7x: The DMA event is the fixed part and I needed to translate that to > local eDMA/sDMA events and in eDMA I needed to pick a channel based on > the translated event. > > The same router code works for eDMA and sDMA in dra7, while the two DMA > engines are different in their internal works. > > At the end of the day I only needed to plug the DMA event routers and > there is no change in the DMA drivers at all. > Please tell me whether I'm wrong but for am335x/am437x both DMA Channels and events are given by DT. I believe IP Spec provides the mapping for the channel (this is what you call fixed channel) and DMA router event is selected randomly within the DT. As for dra7 events comes from DT but channel is selected though out local algorithm. IP Spec defines DMA event muxer mapping. At my opinion my router is more closed to dra7. IP Spec defines event mapping. Nonetheless the DMA has a fixed allocated mapping. Using DMA alone DT has to provide channel number. In router mode this number doesn't matter since router makes the routing from fixed event to channel. However router needs to know which channel will be assign to event: any random channel is allowed. I'm pretty sure I can mimic what has been for DRA7 related to DMA Channel allocation however it seems to be this is aside DMA engine. This kind of implementation forbid the use of DMA and DMA router at the same time : I remember you already raise the point. If a DMA channel is requested DMA router is not aware of this allocation. This is the idea of my custom API which relies on get_any_slave_channel(). Using DT to assign channel seems not a good idea either as I lost router's benefice. BTW I need the channel ID within router. Looking at core or of_dma_router_xlate() I don't really know how to do it a generic manner. Ideas are welcomed > I'm happy to answer any questions you might have, if I can. > You will be happy then. > - Péter > Thanks Py -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 2017-08-24 16:03, Pierre Yves MORDRET wrote: > Please tell me whether I'm wrong but for am335x/am437x both DMA Channels and > events are given by DT. I believe IP Spec provides the mapping for the channel > (this is what you call fixed channel) and DMA router event is selected randomly > within the DT. In our eDMA the channel number and the DMA event number is locked together. Event 17 can only be serviced by eDMA channel 17. The am335x/am437x DMA crossbar is implemented on every DMA event lane and by changing it you can select different event to be routed to the given event number. The eDMA can handle up to 64 unique DMA events, however for each of this 64 events we have a crossbar which allows to select from up to 63 other sources. I can not allocate the eDMA channel in the crossbar as I can not be sure that I will not step on some other driver's toe and take a channel (event) which someone want to use with it's default purpose. So the binding includes the channel number and the crossbar mapped event ID. Oh, and this eDMA limitation is the main source of my hassle with the mem-to-mem channel: in DT we must tell what channels can be used for memcpy - the ones that are not used on the board (usually we pick i2c since we don't support DMA with that). > As for dra7 events comes from DT but channel is selected though out local > algorithm. IP Spec defines DMA event muxer mapping. Here it is again a different story. The crossbar in dra7 can handle 256 incoming events and it can map any of these events to the 64 eDMA event number (or channel) or if it is front of the sDMA we can select the sDMA event number from the 128 it can handle. Here we only use the event number in the crossbar node as we will pick the eDMA event, which is going to be correspond to the eDMA channel. > At my opinion my router is more closed to dra7. IP Spec defines event mapping. > Nonetheless the DMA has a fixed allocated mapping. Using DMA alone DT has to > provide channel number. In router mode this number doesn't matter since router > makes the routing from fixed event to channel. However router needs to know > which channel will be assign to event: any random channel is allowed. In dra7 both eDMA and sDMA have pre-allocated channel mapping. Before my DMA event router support we had no way to use non default mapped events (we did had DMA support fro HSMMC and probably other stuff with direct DMA node). When I had the DMA router, first I added the node for it, then I have converted all existing nodes to use the DMA event router node for the DMA binding. After that I could add the nodes for the peripherals needing non default mapped DMA requests. We don't allow direct DMA binding for dra7 class of devices at all, everything needs to go via the xbars. I would not bother with mixed use case here, I would just make it mandatory to go via the router. Again, with the eDMA I needed to 'mask' out the events/channels which we want to use for memcpy. Define the memcpy channels in the eDMA node and mask the same events in the crossbar as well. > I'm pretty sure I can mimic what has been for DRA7 related to DMA Channel > allocation however it seems to be this is aside DMA engine. This kind of > implementation forbid the use of DMA and DMA router at the same time : I > remember you already raise the point. If a DMA channel is requested DMA router > is not aware of this allocation. This is the idea of my custom API which relies > on get_any_slave_channel(). I think you must not allow mixed use for the slave channels. The none slave (memcpy) is still a pain, but you have managed it somehow in the past. > Using DT to assign channel seems not a good idea either as I lost router's benefice. > > BTW I need the channel ID within router. Your use case is pretty close to the dra7 one we have. > Looking at core or of_dma_router_xlate() I don't really know how to do it a > generic manner. You construct the dmaspec for the DMA, which will include the channel number you have mapped the event to. If the DMA driver needs the channel number _and_ the event number then both, but based on your description your DMA engine itself is close to the eDMA of ours than the sDMA. - Péter -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/28/2017 01:48 PM, Peter Ujfalusi wrote: > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > Thanks for your invaluable inputs. This is more clear now and revise my driver > > I would not bother with mixed use case here, I would just make it > mandatory to go via the router. > I couldn't agree more. But it seems to me there is a flaw here. This should be possible though. I will likely look at that later on. A new revision of this driver will come soon ;) > > - Péter > Py -- To unsubscribe from this list: send the line "unsubscribe dmaengine" 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/Kconfig b/drivers/dma/Kconfig index fa8f9c0..34d9088 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -477,6 +477,15 @@ config STM32_DMA If you have a board based on such a MCU and wish to use DMA say Y here. +config STM32_DMAMUX + bool "STMicroelectronics STM32 dma multiplexer support" + depends on STM32_DMA || COMPILE_TEST + help + Enable support for the on-chip DMA multiplexer on STMicroelectronics + STM32 MCUs. + If you have a board based on such a MCU and wish to use DMAMUX say Y + here. + config S3C24XX_DMAC bool "Samsung S3C24XX DMA support" depends on ARCH_S3C24XX || COMPILE_TEST diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index d12ab29..96bd47e 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -58,6 +58,7 @@ obj-$(CONFIG_RENESAS_DMA) += sh/ obj-$(CONFIG_SIRF_DMA) += sirf-dma.o obj-$(CONFIG_STE_DMA40) += ste_dma40.o ste_dma40_ll.o obj-$(CONFIG_STM32_DMA) += stm32-dma.o +obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o obj-$(CONFIG_S3C24XX_DMAC) += s3c24xx-dma.o obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o obj-$(CONFIG_TEGRA20_APB_DMA) += tegra20-apb-dma.o diff --git a/drivers/dma/stm32-dmamux.c b/drivers/dma/stm32-dmamux.c new file mode 100644 index 0000000..d2756d5 --- /dev/null +++ b/drivers/dma/stm32-dmamux.c @@ -0,0 +1,253 @@ +/* + * DMA Router driver for STM32 DMA MUX + * + * Copyright (C) 2017 M'Boumba Cedric Madianga <cedric.madianga@gmail.com> + * + * Based on TI DMA Crossbar driver + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/of_device.h> +#include <linux/of_dma.h> +#include <linux/reset.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/dma/stm32-dmamux.h> + +#define STM32_DMAMUX_CCR(x) (0x4 * (x)) +#define STM32_DMAMUX_MAX_CHANNELS 32 +#define STM32_DMAMUX_MAX_REQUESTS 255 + +struct stm32_dmamux { + u32 request; + u32 chan_id; + bool busy; +}; + +struct stm32_dmamux_data { + struct dma_router dmarouter; + struct clk *clk; + struct reset_control *rst; + void __iomem *iomem; + u32 dmamux_requests; /* number of DMA requests connected to DMAMUX */ + u32 dmamux_channels; /* Number of DMA channels supported */ + spinlock_t lock; /* Protects register access */ +}; + +static inline u32 stm32_dmamux_read(void __iomem *iomem, u32 reg) +{ + return readl_relaxed(iomem + reg); +} + +static inline void stm32_dmamux_write(void __iomem *iomem, u32 reg, u32 val) +{ + writel_relaxed(val, iomem + reg); +} + +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id) +{ + struct stm32_dmamux_data *dmamux = dev_get_drvdata(dev); + struct stm32_dmamux *mux = route_data; + u32 request = mux->request; + unsigned long flags; + int ret; + + if (chan_id >= dmamux->dmamux_channels) { + dev_err(dev, "invalid channel id\n"); + return -EINVAL; + } + + /* Set dma request */ + spin_lock_irqsave(&dmamux->lock, flags); + if (!IS_ERR(dmamux->clk)) { + ret = clk_enable(dmamux->clk); + if (ret < 0) { + spin_unlock_irqrestore(&dmamux->lock, flags); + dev_err(dev, "clk_prep_enable issue: %d\n", ret); + return ret; + } + } + + stm32_dmamux_write(dmamux->iomem, STM32_DMAMUX_CCR(chan_id), request); + + mux->chan_id = chan_id; + mux->busy = true; + spin_unlock_irqrestore(&dmamux->lock, flags); + + dev_dbg(dev, "Mapping dma-router%dchan%d to request%d\n", dev->id, + mux->chan_id, mux->request); + + return 0; +} + +static void stm32_dmamux_free(struct device *dev, void *route_data) +{ + struct stm32_dmamux_data *dmamux = dev_get_drvdata(dev); + struct stm32_dmamux *mux = route_data; + unsigned long flags; + + /* Clear dma request */ + spin_lock_irqsave(&dmamux->lock, flags); + if (!mux->busy) { + spin_unlock_irqrestore(&dmamux->lock, flags); + goto end; + } + + stm32_dmamux_write(dmamux->iomem, STM32_DMAMUX_CCR(mux->chan_id), 0); + if (!IS_ERR(dmamux->clk)) + clk_disable(dmamux->clk); + spin_unlock_irqrestore(&dmamux->lock, flags); + + dev_dbg(dev, "Unmapping dma-router%dchan%d (was routed to request%d)\n", + dev->id, mux->chan_id, mux->request); + +end: + kfree(mux); +} + +static void *stm32_dmamux_route_allocate(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct platform_device *pdev = of_find_device_by_node(ofdma->of_node); + struct stm32_dmamux_data *dmamux = platform_get_drvdata(pdev); + struct stm32_dmamux *mux; + + if (dma_spec->args_count != 3) { + dev_err(&pdev->dev, "invalid number of dma mux args\n"); + return ERR_PTR(-EINVAL); + } + + if (dma_spec->args[0] > dmamux->dmamux_requests) { + dev_err(&pdev->dev, "invalid mux request number: %d\n", + dma_spec->args[0]); + return ERR_PTR(-EINVAL); + } + + /* The of_node_put() will be done in of_dma_router_xlate function */ + dma_spec->np = of_parse_phandle(ofdma->of_node, "dma-masters", 0); + if (!dma_spec->np) { + dev_err(&pdev->dev, "can't get dma master\n"); + return ERR_PTR(-EINVAL); + } + + mux = kzalloc(sizeof(*mux), GFP_KERNEL); + if (!mux) { + of_node_put(dma_spec->np); + return ERR_PTR(-ENOMEM); + } + mux->request = dma_spec->args[0]; + + dma_spec->args[3] = dma_spec->args[2]; + dma_spec->args[2] = dma_spec->args[1]; + dma_spec->args[1] = 0; + dma_spec->args[0] = 0; + dma_spec->args_count = 4; + + return mux; +} + +static int stm32_dmamux_probe(struct platform_device *pdev) +{ + struct device_node *node = pdev->dev.of_node; + struct device_node *dma_node; + struct stm32_dmamux_data *stm32_dmamux; + struct resource *res; + void __iomem *iomem; + int i, ret; + + if (!node) + return -ENODEV; + + stm32_dmamux = devm_kzalloc(&pdev->dev, sizeof(*stm32_dmamux), + GFP_KERNEL); + if (!stm32_dmamux) + return -ENOMEM; + + dma_node = of_parse_phandle(node, "dma-masters", 0); + if (!dma_node) { + dev_err(&pdev->dev, "Can't get DMA master node\n"); + return -ENODEV; + } + + if (device_property_read_u32(&pdev->dev, "dma-channels", + &stm32_dmamux->dmamux_channels)) + stm32_dmamux->dmamux_channels = STM32_DMAMUX_MAX_CHANNELS; + + if (device_property_read_u32(&pdev->dev, "dma-requests", + &stm32_dmamux->dmamux_requests)) + stm32_dmamux->dmamux_requests = STM32_DMAMUX_MAX_REQUESTS; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; + + iomem = devm_ioremap_resource(&pdev->dev, res); + if (!iomem) + return -ENOMEM; + + spin_lock_init(&stm32_dmamux->lock); + + stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(stm32_dmamux->clk)) { + dev_info(&pdev->dev, "Missing controller clock\n"); + return PTR_ERR(stm32_dmamux->clk); + } + + stm32_dmamux->rst = devm_reset_control_get(&pdev->dev, NULL); + if (!IS_ERR(stm32_dmamux->rst)) { + reset_control_assert(stm32_dmamux->rst); + udelay(2); + reset_control_deassert(stm32_dmamux->rst); + } + + stm32_dmamux->iomem = iomem; + stm32_dmamux->dmarouter.dev = &pdev->dev; + stm32_dmamux->dmarouter.route_free = stm32_dmamux_free; + + platform_set_drvdata(pdev, stm32_dmamux); + + if (!IS_ERR(stm32_dmamux->clk)) { + ret = clk_prepare_enable(stm32_dmamux->clk); + if (ret < 0) { + dev_err(&pdev->dev, "clk_prep_enable issue: %d\n", ret); + return ret; + } + } + + /* Reset the dmamux */ + for (i = 0; i < stm32_dmamux->dmamux_channels; i++) + stm32_dmamux_write(stm32_dmamux->iomem, STM32_DMAMUX_CCR(i), 0); + + if (!IS_ERR(stm32_dmamux->clk)) + clk_disable(stm32_dmamux->clk); + + return of_dma_router_register(node, stm32_dmamux_route_allocate, + &stm32_dmamux->dmarouter); +} + +static const struct of_device_id stm32_dmamux_match[] = { + { .compatible = "st,stm32h7-dmamux" }, + {}, +}; + +static struct platform_driver stm32_dmamux_driver = { + .probe = stm32_dmamux_probe, + .driver = { + .name = "stm32-dmamux", + .of_match_table = stm32_dmamux_match, + }, +}; + +static int __init stm32_dmamux_init(void) +{ + return platform_driver_register(&stm32_dmamux_driver); +} +arch_initcall(stm32_dmamux_init); diff --git a/include/linux/dma/stm32-dmamux.h b/include/linux/dma/stm32-dmamux.h new file mode 100644 index 0000000..3a4eae1 --- /dev/null +++ b/include/linux/dma/stm32-dmamux.h @@ -0,0 +1,21 @@ +/* + * stm32-dmamux.h + * + * Copyright (C) M'Boumba Cedric Madianga 2017 + * Author: M'Boumba Cedric Madianga <cedric.madianga@gmail.com> + * License terms: GNU General Public License (GPL), version 2 + */ + +#ifndef __DMA_STM32_DMAMUX_H +#define __DMA_STM32_DMAMUX_H + +#if defined(CONFIG_STM32_DMAMUX) +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id); +#else +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id) +{ + return -ENODEV; +} +#endif /* CONFIG_STM32_DMAMUX */ + +#endif /* __DMA_STM32_DMAMUX_H */