| Message ID | 1471347080-1411-1-git-send-email-Eugeniy.Paltsev@synopsys.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Eugeniy,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc2 next-20160815]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Eugeniy-Paltsev/DW-Read-is_memcpy-and-is_nollp-property-from-device-tree/20160816-193459
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All errors (new ones prefixed by >>):
drivers/dma/dw/platform.c: In function 'dw_dma_parse_dt':
>> drivers/dma/dw/platform.c:136:8: error: 'struct dw_dma_platform_data' has no member named 'is_nollp'
pdata->is_nollp = true;
^
vim +136 drivers/dma/dw/platform.c
130 pdata->is_private = true;
131
132 if (of_property_read_bool(np, "is_memcpy"))
133 pdata->is_memcpy = true;
134
135 if (of_property_read_bool(np, "is_nollp"))
> 136 pdata->is_nollp = true;
137
138 if (!of_property_read_u32(np, "chan_allocation_order", &tmp))
139 pdata->chan_allocation_order = (unsigned char)tmp;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, 2016-08-16 at 14:31 +0300, Eugeniy Paltsev wrote: > DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: some > Intel > devices has no memcpy support") and 30cb2639 ("dmaengine: dw: don't > override > platform data with autocfg") commits. I'm not sure that word 'broken' is a correct one here. Is the platform code using this driver in the upstream already? If so, where is it located? > > * After df5c7386 commit "DMA_MEMCPY" capability option doesn't get set > correctly in platform driver version. > * After 30cb2639 commit "nollp" parameters don't get set correctly in > platform driver version. > > This happens because in old driver version there are three sources of > parameters: pdata, device tree and autoconfig hardware registers. Some > parameters were read from pdata and others from autoconfig hardware > registers. If pdata was absent some pdata structure fields were filled > with parameters from device tree. > But 30cb2639 commit disabled overriding pdata with autocfg, so if we > use platform driver version without pdata some parameters will not be > set. > This leads to inoperability of DW DMAC. My suggestion is still the same, i.e. split platform data to actual hardware properties and platform quirks. We might be able to use quirks even in case of auto configuration. > > This patch adds reading missed parameters from device tree. > > Note there's a prerequisite http://www.spinics.net/lists/dmaengine/msg > 10682.html > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > drivers/dma/dw/platform.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c > index 5bda0eb..2712602 100644 > --- a/drivers/dma/dw/platform.c > +++ b/drivers/dma/dw/platform.c > @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev) > if (of_property_read_bool(np, "is_private")) > pdata->is_private = true; > > + if (of_property_read_bool(np, "is_memcpy")) > + pdata->is_memcpy = true; > + > + if (of_property_read_bool(np, "is_nollp")) > + pdata->is_nollp = true; I'm pretty sure this one (besides that fact that it misses documentation update and '-' instead of '_' as ordered by DT policy) is unacceptable in DT since it represents *OS related* quirks. (Btw, is_private is also should not be there in the first place) Rob Herring (Cc'ed) might shed a light how to proceed in this case. > + > if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) > pdata->chan_allocation_order = (unsigned char)tmp; >
T24gRnJpLCAyMDE2LTA4LTE5IGF0IDE3OjM5ICswMzAwLCBBbmR5IFNoZXZjaGVua28gd3JvdGU6 DQo+IE9uIFR1ZSwgMjAxNi0wOC0xNiBhdCAxNDozMSArMDMwMCwgRXVnZW5peSBQYWx0c2V2IHdy b3RlOg0KPiA+IA0KPiA+IERXIERNQUMgb24gQVJDIFNEUCBiZWNhbWUgYnJva2VuIGFmdGVyIGRm NWM3Mzg2ICgiZG1hZW5naW5lOiBkdzoNCj4gPiBzb21lDQo+ID4gSW50ZWwNCj4gPiBkZXZpY2Vz IGhhcyBubyBtZW1jcHkgc3VwcG9ydCIpIGFuZCAzMGNiMjYzOSAoImRtYWVuZ2luZTogZHc6IGRv bid0DQo+ID4gb3ZlcnJpZGUNCj4gPiBwbGF0Zm9ybSBkYXRhIHdpdGggYXV0b2NmZyIpIGNvbW1p dHMuDQo+IEknbSBub3Qgc3VyZSB0aGF0IHdvcmQgJ2Jyb2tlbicgaXMgYSBjb3JyZWN0IG9uZSBo ZXJlLiBJcyB0aGUNCj4gcGxhdGZvcm0NCj4gY29kZSB1c2luZyB0aGlzIGRyaXZlciBpbiB0aGUg dXBzdHJlYW0gYWxyZWFkeT8gSWYgc28sIHdoZXJlIGlzIGl0DQo+IGxvY2F0ZWQ/DQo+IA0KSSdt IG5vdCBzdXJlIGlzIGl0LCBidXQsIGF0IGxlYXN0LCBpdCBjaGFuZ2VkIGRyaXZlciBiZWhhdmlv ciBmb3IgQVJDDQpTRFAgYm9hcmRzLg0KPiA+IA0KPiA+IA0KPiA+ICogQWZ0ZXIgZGY1YzczODYg Y29tbWl0ICJETUFfTUVNQ1BZIiBjYXBhYmlsaXR5IG9wdGlvbiBkb2Vzbid0IGdldA0KPiA+IHNl dA0KPiA+IGNvcnJlY3RseSBpbiBwbGF0Zm9ybSBkcml2ZXIgdmVyc2lvbi4NCj4gPiAqIEFmdGVy IDMwY2IyNjM5IGNvbW1pdCAibm9sbHAiIHBhcmFtZXRlcnMgZG9uJ3QgZ2V0IHNldCBjb3JyZWN0 bHkNCj4gPiBpbg0KPiA+IHBsYXRmb3JtIGRyaXZlciB2ZXJzaW9uLg0KPiA+IA0KPiA+IA0KPiA+ IFRoaXMgaGFwcGVucyBiZWNhdXNlIGluIG9sZCBkcml2ZXIgdmVyc2lvbiB0aGVyZSBhcmUgdGhy ZWUgc291cmNlcw0KPiA+IG9mDQo+ID4gcGFyYW1ldGVyczogcGRhdGEsIGRldmljZSB0cmVlIGFu ZCBhdXRvY29uZmlnIGhhcmR3YXJlIHJlZ2lzdGVycy4NCj4gPiBTb21lDQo+ID4gcGFyYW1ldGVy cyB3ZXJlIHJlYWQgZnJvbSBwZGF0YSBhbmQgb3RoZXJzIGZyb20gYXV0b2NvbmZpZyBoYXJkd2Fy ZQ0KPiA+IHJlZ2lzdGVycy4gSWYgcGRhdGEgd2FzIGFic2VudCBzb21lIHBkYXRhIHN0cnVjdHVy ZSBmaWVsZHMgd2VyZQ0KPiA+IGZpbGxlZA0KPiA+IHdpdGggcGFyYW1ldGVycyBmcm9tIGRldmlj ZSB0cmVlLg0KPiANCj4gPiANCj4gPiBCdXQgMzBjYjI2MzkgY29tbWl0IGRpc2FibGVkIG92ZXJy aWRpbmcgcGRhdGEgd2l0aCBhdXRvY2ZnLCBzbyBpZg0KPiA+IHdlDQo+ID4gdXNlIHBsYXRmb3Jt IGRyaXZlciB2ZXJzaW9uIHdpdGhvdXQgcGRhdGEgc29tZSBwYXJhbWV0ZXJzIHdpbGwgbm90DQo+ ID4gYmUNCj4gPiBzZXQuDQo+ID4gVGhpcyBsZWFkcyB0byBpbm9wZXJhYmlsaXR5IG9mIERXIERN QUMuDQo+IE15IHN1Z2dlc3Rpb24gaXMgc3RpbGwgdGhlIHNhbWUsIGkuZS4gc3BsaXQgcGxhdGZv cm0gZGF0YSB0byBhY3R1YWwNCj4gaGFyZHdhcmUgcHJvcGVydGllcyBhbmQgcGxhdGZvcm0gcXVp cmtzLiBXZSBtaWdodCBiZSBhYmxlIHRvIHVzZQ0KPiBxdWlya3MNCj4gZXZlbiBpbiBjYXNlIG9m IGF1dG8gY29uZmlndXJhdGlvbi4NCkRvIHlvdSBoYXZlIGFueSBpZGVhIGFib3V0IGJldHRlciB3 YXkgdG8gZG8gaXQ/DQpEbyB5b3Ugc3VnZ2VzdCB0byBzcGxpdCBwZGF0YSBzdHJ1Y3R1cmUgaW4g dHdvIGRpZmZlcmVudCBzdHJ1Y3R1cmVzPw0KPiANCj4gPiANCj4gPiANCj4gPiBUaGlzIHBhdGNo IGFkZHMgcmVhZGluZyBtaXNzZWQgcGFyYW1ldGVycyBmcm9tIGRldmljZSB0cmVlLg0KPiA+IA0K PiA+IE5vdGUgdGhlcmUncyBhIHByZXJlcXVpc2l0ZSBodHRwOi8vd3d3LnNwaW5pY3MubmV0L2xp c3RzL2RtYWVuZ2luZS8NCj4gPiBtc2cNCj4gPiAxMDY4Mi5odG1sDQo+ID4gDQo+ID4gU2lnbmVk LW9mZi1ieTogRXVnZW5peSBQYWx0c2V2IDxFdWdlbml5LlBhbHRzZXZAc3lub3BzeXMuY29tPg0K PiA+IC0tLQ0KPiA+IMKgZHJpdmVycy9kbWEvZHcvcGxhdGZvcm0uYyB8IDYgKysrKysrDQo+ID4g wqAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspDQo+ID4gDQo+ID4gZGlmZiAtLWdpdCBh L2RyaXZlcnMvZG1hL2R3L3BsYXRmb3JtLmMgYi9kcml2ZXJzL2RtYS9kdy9wbGF0Zm9ybS5jDQo+ ID4gaW5kZXggNWJkYTBlYi4uMjcxMjYwMiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2RtYS9k dy9wbGF0Zm9ybS5jDQo+ID4gKysrIGIvZHJpdmVycy9kbWEvZHcvcGxhdGZvcm0uYw0KPiA+IEBA IC0xMjksNiArMTI5LDEyIEBAIGR3X2RtYV9wYXJzZV9kdChzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNl ICpwZGV2KQ0KPiA+IMKgCWlmIChvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wobnAsICJpc19wcml2YXRl IikpDQo+ID4gwqAJCXBkYXRhLT5pc19wcml2YXRlID0gdHJ1ZTsNCj4gPiDCoA0KPiA+ICsJaWYg KG9mX3Byb3BlcnR5X3JlYWRfYm9vbChucCwgImlzX21lbWNweSIpKQ0KPiA+ICsJCXBkYXRhLT5p c19tZW1jcHkgPSB0cnVlOw0KPiA+ICsNCj4gPiArCWlmIChvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wo bnAsICJpc19ub2xscCIpKQ0KPiA+ICsJCXBkYXRhLT5pc19ub2xscCA9IHRydWU7DQo+IEknbSBw cmV0dHkgc3VyZSB0aGlzIG9uZSAoYmVzaWRlcyB0aGF0IGZhY3QgdGhhdCBpdCBtaXNzZXMNCj4g ZG9jdW1lbnRhdGlvbg0KPiB1cGRhdGUgYW5kICctJyBpbnN0ZWFkIG9mICdfJyBhcyBvcmRlcmVk IGJ5IERUIHBvbGljeSkgaXMNCj4gdW5hY2NlcHRhYmxlDQo+IGluIERUIHNpbmNlIGl0IHJlcHJl c2VudHMgKk9TIHJlbGF0ZWQqIHF1aXJrcy4gKEJ0dywgaXNfcHJpdmF0ZSBpcw0KPiBhbHNvDQo+ IHNob3VsZCBub3QgYmUgdGhlcmUgaW4gdGhlIGZpcnN0IHBsYWNlKQ0KQ291bGQgeW91IHBvc3Np Ymx5IHRlbGwgbWUsIHdoeSB5b3UgY2FsbCB0aGlzIHF1aXJrcyAqT1MgcmVsYXRlZCogPw0KRm9y IGV4YW1wbGU6DQpJZiBJIGtub3csIHdoYXQgRE1BQyBpbiBhbnkgY2hpcCBvbiBhbnkgYm9hcmQg ZG9lc24ndCBzdXBwb3J0IG1lbW9yeS0NCnRvLW1lbW9yeSB0cmFuc2ZlcnMsIEkgY2FuIGRpc2Fi bGUgImlzX21lbWNweSIgaW4gdGhpcyBib2FyZCAuZHRzIGZpbGUuDQpBbSBJIHdyb25nP8KgDQo+ IA0KPiBSb2IgSGVycmluZyAoQ2MnZWQpIG1pZ2h0IHNoZWQgYSBsaWdodCBob3cgdG8gcHJvY2Vl ZCBpbiB0aGlzIGNhc2UuDQo+IA0KPiA+IA0KPiA+ICsNCj4gPiDCoAlpZiAoIW9mX3Byb3BlcnR5 X3JlYWRfdTMyKG5wLCAiY2hhbl9hbGxvY2F0aW9uX29yZGVyIiwNCj4gPiAmdG1wKSkNCj4gPiDC oAkJcGRhdGEtPmNoYW5fYWxsb2NhdGlvbl9vcmRlciA9ICh1bnNpZ25lZCBjaGFyKXRtcDsNCj4g PiDCoA0KLS0gDQrCoFBhbHRzZXYgRXVnZW5peQ0K -- 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, 2016-08-23 at 15:14 +0000, Eugeniy Paltsev wrote: > DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: > > > some Intel devices has no memcpy support") and 30cb2639 > > > ("dmaengine: dw: don't override platform data with autocfg") > > > commits. > > I'm not sure that word 'broken' is a correct one here. Is the > > platform > > code using this driver in the upstream already? If so, where is it > > located? > > > I'm not sure is it, but, at least, it changed driver behavior for ARC > SDP boards. The rule of common sense here: if it was never upstreamed it has never been broken. I hardly remember any user of DW DMAC by ARC architecture in upstream. > > > But 30cb2639 commit disabled overriding pdata with autocfg, so if > > > we use platform driver version without pdata some parameters will > > > not be set. This leads to inoperability of DW DMAC. > > My suggestion is still the same, i.e. split platform data to actual > > hardware properties and platform quirks. We might be able to use > > quirks > > even in case of auto configuration. > Do you have any idea about better way to do it? > Do you suggest to split pdata structure in two different structures There might be at least couple of ways how to implement this. 1. Convert booleans to bits in one variable (let's say unsigned int quirks). 2. Split all quirks to separate quirks to something like struct dw_dma_platform_quirks. By obvious reasons I think first solution might be better. > > > + if (of_property_read_bool(np, "is_memcpy")) > > > + pdata->is_memcpy = true; > > > + > > > + if (of_property_read_bool(np, "is_nollp")) > > > + pdata->is_nollp = true; > > I'm pretty sure this one (besides that fact that it misses > > documentation update and '-' instead of '_' as ordered by DT > > policy) is unacceptable in DT since it represents *OS related* > > quirks. (Btw,is_private is also should not be there in the first > > place) > Could you possibly tell me, why you call this quirks *OS related* ? > For example: > If I know, what DMAC in any chip on any board doesn't support memory- > to-memory transfers, I can disable "is_memcpy" in this board .dts > file. > Am I wrong? Some of the properties are set or unset due to support in the driver and / or issues of the hardware _related_ to the driver in question. Though if anyone has different opinion I would appreciate to listen to.
On 08/23/2016 10:02 AM, Andy Shevchenko wrote: > On Tue, 2016-08-23 at 15:14 +0000, Eugeniy Paltsev wrote: > >> DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: >>>> some Intel devices has no memcpy support") and 30cb2639 >>>> ("dmaengine: dw: don't override platform data with autocfg") >>>> commits. >>> I'm not sure that word 'broken' is a correct one here. Is the >>> platform >>> code using this driver in the upstream already? If so, where is it >>> located? >>> >> I'm not sure is it, but, at least, it changed driver behavior for ARC >> SDP boards. > The rule of common sense here: if it was never upstreamed it has never > been broken. Right ! > I hardly remember any user of DW DMAC by ARC architecture in upstream. The ARC SDP platform is provided by arch/arc/plat-axs and arch/arc/boot/ax* The IP Proto-typing kit folks here would just add a DT binding in there and things would just work out of the box - and that stopped recently - hence the notion of broken. But I agree one can't fix what can't be seen as broken. I just intervened to make this comment - I'm sure you and Eugeniy can agree on a workable solution. Thx, -Vineet -- 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/dw/platform.c b/drivers/dma/dw/platform.c index 5bda0eb..2712602 100644 --- a/drivers/dma/dw/platform.c +++ b/drivers/dma/dw/platform.c @@ -129,6 +129,12 @@ dw_dma_parse_dt(struct platform_device *pdev) if (of_property_read_bool(np, "is_private")) pdata->is_private = true; + if (of_property_read_bool(np, "is_memcpy")) + pdata->is_memcpy = true; + + if (of_property_read_bool(np, "is_nollp")) + pdata->is_nollp = true; + if (!of_property_read_u32(np, "chan_allocation_order", &tmp)) pdata->chan_allocation_order = (unsigned char)tmp;
DW DMAC on ARC SDP became broken after df5c7386 ("dmaengine: dw: some Intel devices has no memcpy support") and 30cb2639 ("dmaengine: dw: don't override platform data with autocfg") commits. * After df5c7386 commit "DMA_MEMCPY" capability option doesn't get set correctly in platform driver version. * After 30cb2639 commit "nollp" parameters don't get set correctly in platform driver version. This happens because in old driver version there are three sources of parameters: pdata, device tree and autoconfig hardware registers. Some parameters were read from pdata and others from autoconfig hardware registers. If pdata was absent some pdata structure fields were filled with parameters from device tree. But 30cb2639 commit disabled overriding pdata with autocfg, so if we use platform driver version without pdata some parameters will not be set. This leads to inoperability of DW DMAC. This patch adds reading missed parameters from device tree. Note there's a prerequisite http://www.spinics.net/lists/dmaengine/msg10682.html Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- drivers/dma/dw/platform.c | 6 ++++++ 1 file changed, 6 insertions(+)