From mboxrd@z Thu Jan 1 00:00:00 1970 From: andriy.shevchenko@intel.com (Shevchenko, Andriy) Date: Fri, 12 Oct 2012 14:41:55 +0000 Subject: [PATCH 2/3] dmaengine: dw_dmac: Enhance device tree support In-Reply-To: References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> <1350048519.10584.158.camel@smile> Message-ID: <1350052912.10584.167.camel@smile> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 2012-10-12 at 19:36 +0530, Viresh Kumar wrote: > On 12 October 2012 18:58, Shevchenko, Andriy > wrote: > > On Fri, 2012-10-12 at 11:14 +0530, Viresh Kumar wrote: > > >> + if (!of_property_read_u32(np, "nr_masters", &val)) { > >> + if (val > 4) > > I thought once that we might introduce constant like #define > > DW_MAX_AHB_MASTERS 4. It seems a bit useless because hw is designed for > > that number anyway, but maybe you have another opinion. > > Not everyone have four masters in there SoC configurations. So its better > to export correct values. I meant is to use that constant instead of hard coding 4 everywhere. It's a maximum value, not the SoC specific. > >> + if (!of_property_read_u32_array(np, "data_width", arr, > >> + pdata->nr_masters)) > >> + for (count = 0; count < pdata->nr_masters; count++) > >> + pdata->data_width[count] = arr[count]; > > Ah, it would be nice to have of_property_read_u8_array and so on... > > :) > Maybe yes. I don't want to do it with this patchset, as that will take more > time to go through maintainers. Agreed, the comment just for future. I don't know if there any other users of *_u8/u16_array(). > > it will probably require to split this part in a separate function and > > calculate count of slave_info children before pdata memory allocation. > > Liked the idea partially. Check my new implementation in fixup for all > these comments: Let me see. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > @@ -1516,7 +1516,7 @@ __devinit dw_dma_parse_dt(struct platform_device *pdev) > struct device_node *sn, *cn, *np = pdev->dev.of_node; > struct dw_dma_platform_data *pdata; > struct dw_dma_slave *sd; > - u32 count, val, arr[4]; > + u32 val, arr[4]; what about to use tmp name instead of val? It's really minor, but I think val name is little bit unrelated to a loop counter. > @@ -1533,63 +1533,57 @@ __devinit dw_dma_parse_dt(struct platform_device *pdev) > - count = 0; > + pdata->sd = sd; > + pdata->sd_count = val; > + > for_each_child_of_node(sn, cn) { > - of_property_read_string(cn, "bus_id", &sd[count].bus_id); > - of_property_read_u32(cn, "cfg_hi", &sd[count].cfg_hi); > - of_property_read_u32(cn, "cfg_lo", &sd[count].cfg_lo); > + sd->dma_dev = &pdev->dev; > + of_property_read_string(cn, "bus_id", &sd->bus_id); > + of_property_read_u32(cn, "cfg_hi", &sd->cfg_hi); > + of_property_read_u32(cn, "cfg_lo", &sd->cfg_lo); > if (!of_property_read_u32(cn, "src_master", &val)) > - sd[count].src_master = (u8)val; > + sd->src_master = val; > > if (!of_property_read_u32(cn, "dst_master", &val)) > - sd[count].dst_master = (u8)val; > - > - sd[count].dma_dev = &pdev->dev; > - count++; > + sd->dst_master = val; > + sd++; > } This looks good. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Shevchenko, Andriy" Subject: Re: [PATCH 2/3] dmaengine: dw_dmac: Enhance device tree support Date: Fri, 12 Oct 2012 14:41:55 +0000 Message-ID: <1350052912.10584.167.camel@smile> References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> <1350048519.10584.158.camel@smile> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: Content-Language: en-US Content-ID: <32EFD540BB052D47B5F326DB81C205FC@intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Viresh Kumar Cc: "Koul, Vinod" , "linux-kernel@vger.kernel.org" , "spear-devel@list.st.com" , "linux-arm-kernel@lists.infradead.org" , devicetree-discuss List-Id: devicetree@vger.kernel.org T24gRnJpLCAyMDEyLTEwLTEyIGF0IDE5OjM2ICswNTMwLCBWaXJlc2ggS3VtYXIgd3JvdGU6IA0K PiBPbiAxMiBPY3RvYmVyIDIwMTIgMTg6NTgsIFNoZXZjaGVua28sIEFuZHJpeQ0KPiA8YW5kcml5 LnNoZXZjaGVua29AaW50ZWwuY29tPiB3cm90ZToNCj4gPiBPbiBGcmksIDIwMTItMTAtMTIgYXQg MTE6MTQgKzA1MzAsIFZpcmVzaCBLdW1hciB3cm90ZToNCj4gDQo+ID4+ICsgICAgIGlmICghb2Zf cHJvcGVydHlfcmVhZF91MzIobnAsICJucl9tYXN0ZXJzIiwgJnZhbCkpIHsNCj4gPj4gKyAgICAg ICAgICAgICBpZiAodmFsID4gNCkNCj4gPiBJIHRob3VnaHQgb25jZSB0aGF0IHdlIG1pZ2h0IGlu dHJvZHVjZSBjb25zdGFudCBsaWtlICNkZWZpbmUNCj4gPiBEV19NQVhfQUhCX01BU1RFUlMgNC4g SXQgc2VlbXMgYSBiaXQgdXNlbGVzcyBiZWNhdXNlIGh3IGlzIGRlc2lnbmVkIGZvcg0KPiA+IHRo YXQgbnVtYmVyIGFueXdheSwgYnV0IG1heWJlIHlvdSBoYXZlIGFub3RoZXIgb3Bpbmlvbi4NCj4g DQo+IE5vdCBldmVyeW9uZSBoYXZlIGZvdXIgbWFzdGVycyBpbiB0aGVyZSBTb0MgY29uZmlndXJh dGlvbnMuIFNvIGl0cyBiZXR0ZXINCj4gdG8gZXhwb3J0IGNvcnJlY3QgdmFsdWVzLg0KSSBtZWFu dCBpcyB0byB1c2UgdGhhdCBjb25zdGFudCBpbnN0ZWFkIG9mIGhhcmQgY29kaW5nIDQgZXZlcnl3 aGVyZS4NCkl0J3MgYSBtYXhpbXVtIHZhbHVlLCBub3QgdGhlIFNvQyBzcGVjaWZpYy4NCg0KPiA+ PiArICAgICBpZiAoIW9mX3Byb3BlcnR5X3JlYWRfdTMyX2FycmF5KG5wLCAiZGF0YV93aWR0aCIs IGFyciwNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcGRhdGEtPm5yX21hc3Rl cnMpKQ0KPiA+PiArICAgICAgICAgICAgIGZvciAoY291bnQgPSAwOyBjb3VudCA8IHBkYXRhLT5u cl9tYXN0ZXJzOyBjb3VudCsrKQ0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgcGRhdGEtPmRh dGFfd2lkdGhbY291bnRdID0gYXJyW2NvdW50XTsNCj4gPiBBaCwgaXQgd291bGQgYmUgbmljZSB0 byBoYXZlIG9mX3Byb3BlcnR5X3JlYWRfdThfYXJyYXkgYW5kIHNvIG9uLi4uDQo+IA0KPiA6KQ0K PiBNYXliZSB5ZXMuIEkgZG9uJ3Qgd2FudCB0byBkbyBpdCB3aXRoIHRoaXMgcGF0Y2hzZXQsIGFz IHRoYXQgd2lsbCB0YWtlIG1vcmUNCj4gdGltZSB0byBnbyB0aHJvdWdoIG1haW50YWluZXJzLg0K QWdyZWVkLCB0aGUgY29tbWVudCBqdXN0IGZvciBmdXR1cmUuIEkgZG9uJ3Qga25vdyBpZiB0aGVy ZSBhbnkgb3RoZXINCnVzZXJzIG9mICpfdTgvdTE2X2FycmF5KCkuDQoNCj4gPiBpdCB3aWxsIHBy b2JhYmx5IHJlcXVpcmUgdG8gc3BsaXQgdGhpcyBwYXJ0IGluIGEgc2VwYXJhdGUgZnVuY3Rpb24g YW5kDQo+ID4gY2FsY3VsYXRlIGNvdW50IG9mIHNsYXZlX2luZm8gY2hpbGRyZW4gYmVmb3JlIHBk YXRhIG1lbW9yeSBhbGxvY2F0aW9uLg0KPiANCj4gTGlrZWQgdGhlIGlkZWEgcGFydGlhbGx5LiBD aGVjayBteSBuZXcgaW1wbGVtZW50YXRpb24gaW4gZml4dXAgZm9yIGFsbA0KPiB0aGVzZSBjb21t ZW50czoNCkxldCBtZSBzZWUuDQoNCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZG1hL2R3X2RtYWMu YyBiL2RyaXZlcnMvZG1hL2R3X2RtYWMuYw0KDQo+IEBAIC0xNTE2LDcgKzE1MTYsNyBAQCBfX2Rl dmluaXQgZHdfZG1hX3BhcnNlX2R0KHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+ICAJ c3RydWN0IGRldmljZV9ub2RlICpzbiwgKmNuLCAqbnAgPSBwZGV2LT5kZXYub2Zfbm9kZTsNCj4g IAlzdHJ1Y3QgZHdfZG1hX3BsYXRmb3JtX2RhdGEgKnBkYXRhOw0KPiAgCXN0cnVjdCBkd19kbWFf c2xhdmUgKnNkOw0KPiAtCXUzMiBjb3VudCwgdmFsLCBhcnJbNF07DQo+ICsJdTMyIHZhbCwgYXJy WzRdOw0Kd2hhdCBhYm91dCB0byB1c2UgdG1wIG5hbWUgaW5zdGVhZCBvZiB2YWw/IEl0J3MgcmVh bGx5IG1pbm9yLCBidXQgSQ0KdGhpbmsgdmFsIG5hbWUgaXMgbGl0dGxlIGJpdCB1bnJlbGF0ZWQg dG8gYSBsb29wIGNvdW50ZXIuDQoNCj4gQEAgLTE1MzMsNjMgKzE1MzMsNTcgQEAgX19kZXZpbml0 IGR3X2RtYV9wYXJzZV9kdChzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KDQo+IC0JY291 bnQgPSAwOw0KPiArCXBkYXRhLT5zZCA9IHNkOw0KPiArCXBkYXRhLT5zZF9jb3VudCA9IHZhbDsN Cj4gKw0KPiAgCWZvcl9lYWNoX2NoaWxkX29mX25vZGUoc24sIGNuKSB7DQo+IC0JCW9mX3Byb3Bl cnR5X3JlYWRfc3RyaW5nKGNuLCAiYnVzX2lkIiwgJnNkW2NvdW50XS5idXNfaWQpOw0KPiAtCQlv Zl9wcm9wZXJ0eV9yZWFkX3UzMihjbiwgImNmZ19oaSIsICZzZFtjb3VudF0uY2ZnX2hpKTsNCj4g LQkJb2ZfcHJvcGVydHlfcmVhZF91MzIoY24sICJjZmdfbG8iLCAmc2RbY291bnRdLmNmZ19sbyk7 DQo+ICsJCXNkLT5kbWFfZGV2ID0gJnBkZXYtPmRldjsNCj4gKwkJb2ZfcHJvcGVydHlfcmVhZF9z dHJpbmcoY24sICJidXNfaWQiLCAmc2QtPmJ1c19pZCk7DQo+ICsJCW9mX3Byb3BlcnR5X3JlYWRf dTMyKGNuLCAiY2ZnX2hpIiwgJnNkLT5jZmdfaGkpOw0KPiArCQlvZl9wcm9wZXJ0eV9yZWFkX3Uz MihjbiwgImNmZ19sbyIsICZzZC0+Y2ZnX2xvKTsNCj4gIAkJaWYgKCFvZl9wcm9wZXJ0eV9yZWFk X3UzMihjbiwgInNyY19tYXN0ZXIiLCAmdmFsKSkNCj4gLQkJCXNkW2NvdW50XS5zcmNfbWFzdGVy ID0gKHU4KXZhbDsNCj4gKwkJCXNkLT5zcmNfbWFzdGVyID0gdmFsOw0KPiANCj4gIAkJaWYgKCFv Zl9wcm9wZXJ0eV9yZWFkX3UzMihjbiwgImRzdF9tYXN0ZXIiLCAmdmFsKSkNCj4gLQkJCXNkW2Nv dW50XS5kc3RfbWFzdGVyID0gKHU4KXZhbDsNCj4gLQ0KPiAtCQlzZFtjb3VudF0uZG1hX2RldiA9 ICZwZGV2LT5kZXY7DQo+IC0JCWNvdW50Kys7DQo+ICsJCQlzZC0+ZHN0X21hc3RlciA9IHZhbDsN Cj4gKwkJc2QrKzsNCj4gIAl9DQpUaGlzIGxvb2tzIGdvb2QuDQoNCi0tIA0KQW5keSBTaGV2Y2hl bmtvIDxhbmRyaXkuc2hldmNoZW5rb0BpbnRlbC5jb20+DQpJbnRlbCBGaW5sYW5kIE95DQotLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0KSW50ZWwgRmlubGFuZCBPeQpSZWdpc3RlcmVkIEFkZHJlc3M6IFBMIDI4MSwgMDAx ODEgSGVsc2lua2kgCkJ1c2luZXNzIElkZW50aXR5IENvZGU6IDAzNTc2MDYgLSA0IApEb21pY2ls ZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUtbWFpbCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBjb250 YWluIGNvbmZpZGVudGlhbCBtYXRlcmlhbCBmb3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRl ZCByZWNpcGllbnQocykuIEFueSByZXZpZXcgb3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBpcyBz dHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50 LCBwbGVhc2UgY29udGFjdCB0aGUgc2VuZGVyIGFuZCBkZWxldGUgYWxsIGNvcGllcy4K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757937Ab2JLOnT (ORCPT ); Fri, 12 Oct 2012 10:43:19 -0400 Received: from mga01.intel.com ([192.55.52.88]:27283 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756712Ab2JLOnR (ORCPT ); Fri, 12 Oct 2012 10:43:17 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.80,576,1344236400"; d="scan'208";a="233213464" From: "Shevchenko, Andriy" To: Viresh Kumar CC: "Koul, Vinod" , "linux-kernel@vger.kernel.org" , "spear-devel@list.st.com" , "linux-arm-kernel@lists.infradead.org" , devicetree-discuss Subject: Re: [PATCH 2/3] dmaengine: dw_dmac: Enhance device tree support Thread-Topic: [PATCH 2/3] dmaengine: dw_dmac: Enhance device tree support Thread-Index: AQHNqILV4m+3vxUD3EiqzcH7Q9+iIJe1raYA Date: Fri, 12 Oct 2012 14:41:55 +0000 Message-ID: <1350052912.10584.167.camel@smile> References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> <1350048519.10584.158.camel@smile> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.237.72.62] Content-Type: text/plain; charset="utf-8" Content-ID: <32EFD540BB052D47B5F326DB81C205FC@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id q9CEhO3R016054 On Fri, 2012-10-12 at 19:36 +0530, Viresh Kumar wrote: > On 12 October 2012 18:58, Shevchenko, Andriy > wrote: > > On Fri, 2012-10-12 at 11:14 +0530, Viresh Kumar wrote: > > >> + if (!of_property_read_u32(np, "nr_masters", &val)) { > >> + if (val > 4) > > I thought once that we might introduce constant like #define > > DW_MAX_AHB_MASTERS 4. It seems a bit useless because hw is designed for > > that number anyway, but maybe you have another opinion. > > Not everyone have four masters in there SoC configurations. So its better > to export correct values. I meant is to use that constant instead of hard coding 4 everywhere. It's a maximum value, not the SoC specific. > >> + if (!of_property_read_u32_array(np, "data_width", arr, > >> + pdata->nr_masters)) > >> + for (count = 0; count < pdata->nr_masters; count++) > >> + pdata->data_width[count] = arr[count]; > > Ah, it would be nice to have of_property_read_u8_array and so on... > > :) > Maybe yes. I don't want to do it with this patchset, as that will take more > time to go through maintainers. Agreed, the comment just for future. I don't know if there any other users of *_u8/u16_array(). > > it will probably require to split this part in a separate function and > > calculate count of slave_info children before pdata memory allocation. > > Liked the idea partially. Check my new implementation in fixup for all > these comments: Let me see. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > @@ -1516,7 +1516,7 @@ __devinit dw_dma_parse_dt(struct platform_device *pdev) > struct device_node *sn, *cn, *np = pdev->dev.of_node; > struct dw_dma_platform_data *pdata; > struct dw_dma_slave *sd; > - u32 count, val, arr[4]; > + u32 val, arr[4]; what about to use tmp name instead of val? It's really minor, but I think val name is little bit unrelated to a loop counter. > @@ -1533,63 +1533,57 @@ __devinit dw_dma_parse_dt(struct platform_device *pdev) > - count = 0; > + pdata->sd = sd; > + pdata->sd_count = val; > + > for_each_child_of_node(sn, cn) { > - of_property_read_string(cn, "bus_id", &sd[count].bus_id); > - of_property_read_u32(cn, "cfg_hi", &sd[count].cfg_hi); > - of_property_read_u32(cn, "cfg_lo", &sd[count].cfg_lo); > + sd->dma_dev = &pdev->dev; > + of_property_read_string(cn, "bus_id", &sd->bus_id); > + of_property_read_u32(cn, "cfg_hi", &sd->cfg_hi); > + of_property_read_u32(cn, "cfg_lo", &sd->cfg_lo); > if (!of_property_read_u32(cn, "src_master", &val)) > - sd[count].src_master = (u8)val; > + sd->src_master = val; > > if (!of_property_read_u32(cn, "dst_master", &val)) > - sd[count].dst_master = (u8)val; > - > - sd[count].dma_dev = &pdev->dev; > - count++; > + sd->dst_master = val; > + sd++; > } This looks good. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I