From mboxrd@z Thu Jan 1 00:00:00 1970 From: andriy.shevchenko@intel.com (Shevchenko, Andriy) Date: Fri, 12 Oct 2012 13:28:43 +0000 Subject: [PATCH 2/3] dmaengine: dw_dmac: Enhance device tree support In-Reply-To: <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> Message-ID: <1350048519.10584.158.camel@smile> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, 2012-10-12 at 11:14 +0530, Viresh Kumar wrote: > dw_dmac driver already supports device tree but it used to have its platform > data passed the non-DT way. Another portion of comments. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > +static struct dw_dma_platform_data * > +__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]; > + > + if (!np) { > + dev_err(&pdev->dev, "Missing DT data\n"); > + return NULL; > + } > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + if (of_property_read_u32(np, "nr_channels", &pdata->nr_channels)) > + return NULL; > + > + if (of_property_read_bool(np, "is_private")) > + pdata->is_private = true; > + > + if (!of_property_read_u32(np, "chan_allocation_order", > + &val)) Fits one line? > + pdata->chan_allocation_order = (unsigned char)val; do we really need explicit casting here? > + > + if (!of_property_read_u32(np, "chan_priority", &val)) > + pdata->chan_priority = (unsigned char)val; ditto > + > + if (!of_property_read_u32(np, "block_size", &val)) > + pdata->block_size = (unsigned short)val; ditto > + > + 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. > + return NULL; > + > + pdata->nr_masters = (unsigned char)val; > + } > + > + 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... > + > + /* parse slave data */ > + sn = of_find_node_by_name(np, "slave_info"); > + if (!sn) > + return pdata; > + > + count = 0; > + /* calculate number of slaves */ > + for_each_child_of_node(sn, cn) > + count++; of.h: static inline int of_get_child_count(const struct device_node *np) > + > + if (!count) > + return NULL; > + > + sd = devm_kzalloc(&pdev->dev, sizeof(*sd) * count, GFP_KERNEL); > + if (!sd) > + return NULL; > + > + count = 0; > + 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); > + if (!of_property_read_u32(cn, "src_master", &val)) > + sd[count].src_master = (u8)val; Explicit casting? > + > + if (!of_property_read_u32(cn, "dst_master", &val)) > + sd[count].dst_master = (u8)val; ditto > + > + sd[count].dma_dev = &pdev->dev; > + count++; > + } what about to define sd as sd[0]; in the platform_data structure and then use smth like following struct ... *sd = pdata->sd; for_each... { sd->... = ; sd++; } it will probably require to split this part in a separate function and calculate count of slave_info children before pdata memory allocation. > + > + pdata->sd = sd; > + pdata->sd_count = count; > + > + return pdata; > +} -- Andy Shevchenko Intel Finland Oy -- 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 13:28:43 +0000 Message-ID: <1350048519.10584.158.camel@smile> References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> Content-Language: en-US Content-ID: <2EB5CA6FD0272640BF09A738D5796869@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 T24gRnJpLCAyMDEyLTEwLTEyIGF0IDExOjE0ICswNTMwLCBWaXJlc2ggS3VtYXIgd3JvdGU6IA0K PiBkd19kbWFjIGRyaXZlciBhbHJlYWR5IHN1cHBvcnRzIGRldmljZSB0cmVlIGJ1dCBpdCB1c2Vk IHRvIGhhdmUgaXRzIHBsYXRmb3JtDQo+IGRhdGEgcGFzc2VkIHRoZSBub24tRFQgd2F5Lg0KQW5v dGhlciBwb3J0aW9uIG9mIGNvbW1lbnRzLg0KDQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2RtYS9k d19kbWFjLmMgYi9kcml2ZXJzL2RtYS9kd19kbWFjLmMNCg0KPiArc3RhdGljIHN0cnVjdCBkd19k bWFfcGxhdGZvcm1fZGF0YSAqDQo+ICtfX2RldmluaXQgZHdfZG1hX3BhcnNlX2R0KHN0cnVjdCBw bGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+ICt7DQo+ICsJc3RydWN0IGRldmljZV9ub2RlICpzbiwg KmNuLCAqbnAgPSBwZGV2LT5kZXYub2Zfbm9kZTsNCj4gKwlzdHJ1Y3QgZHdfZG1hX3BsYXRmb3Jt X2RhdGEgKnBkYXRhOw0KPiArCXN0cnVjdCBkd19kbWFfc2xhdmUgKnNkOw0KPiArCXUzMiBjb3Vu dCwgdmFsLCBhcnJbNF07DQo+ICsNCj4gKwlpZiAoIW5wKSB7DQo+ICsJCWRldl9lcnIoJnBkZXYt PmRldiwgIk1pc3NpbmcgRFQgZGF0YVxuIik7DQo+ICsJCXJldHVybiBOVUxMOw0KPiArCX0NCj4g Kw0KPiArCXBkYXRhID0gZGV2bV9remFsbG9jKCZwZGV2LT5kZXYsIHNpemVvZigqcGRhdGEpLCBH RlBfS0VSTkVMKTsNCj4gKwlpZiAoIXBkYXRhKQ0KPiArCQlyZXR1cm4gTlVMTDsNCj4gKw0KPiAr CWlmIChvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgIm5yX2NoYW5uZWxzIiwgJnBkYXRhLT5ucl9j aGFubmVscykpDQo+ICsJCXJldHVybiBOVUxMOw0KPiArDQo+ICsJaWYgKG9mX3Byb3BlcnR5X3Jl YWRfYm9vbChucCwgImlzX3ByaXZhdGUiKSkNCj4gKwkJcGRhdGEtPmlzX3ByaXZhdGUgPSB0cnVl Ow0KPiArDQo+ICsJaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgImNoYW5fYWxsb2NhdGlv bl9vcmRlciIsDQo+ICsJCQkJJnZhbCkpDQpGaXRzIG9uZSBsaW5lPw0KDQo+ICsJCXBkYXRhLT5j aGFuX2FsbG9jYXRpb25fb3JkZXIgPSAodW5zaWduZWQgY2hhcil2YWw7DQpkbyB3ZSByZWFsbHkg bmVlZCBleHBsaWNpdCBjYXN0aW5nIGhlcmU/DQoNCj4gKw0KPiArCWlmICghb2ZfcHJvcGVydHlf cmVhZF91MzIobnAsICJjaGFuX3ByaW9yaXR5IiwgJnZhbCkpDQo+ICsJCXBkYXRhLT5jaGFuX3By aW9yaXR5ID0gKHVuc2lnbmVkIGNoYXIpdmFsOw0KZGl0dG8NCg0KPiArDQo+ICsJaWYgKCFvZl9w cm9wZXJ0eV9yZWFkX3UzMihucCwgImJsb2NrX3NpemUiLCAmdmFsKSkNCj4gKwkJcGRhdGEtPmJs b2NrX3NpemUgPSAodW5zaWduZWQgc2hvcnQpdmFsOw0KZGl0dG8gDQo+ICsNCj4gKwlpZiAoIW9m X3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAibnJfbWFzdGVycyIsICZ2YWwpKSB7DQo+ICsJCWlmICh2 YWwgPiA0KQ0KSSB0aG91Z2h0IG9uY2UgdGhhdCB3ZSBtaWdodCBpbnRyb2R1Y2UgY29uc3RhbnQg bGlrZSAjZGVmaW5lDQpEV19NQVhfQUhCX01BU1RFUlMgNC4gSXQgc2VlbXMgYSBiaXQgdXNlbGVz cyBiZWNhdXNlIGh3IGlzIGRlc2lnbmVkIGZvcg0KdGhhdCBudW1iZXIgYW55d2F5LCBidXQgbWF5 YmUgeW91IGhhdmUgYW5vdGhlciBvcGluaW9uLiANCg0KPiArCQkJcmV0dXJuIE5VTEw7DQo+ICsN Cj4gKwkJcGRhdGEtPm5yX21hc3RlcnMgPSAodW5zaWduZWQgY2hhcil2YWw7DQo+ICsJfQ0KPiAr DQo+ICsJaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMl9hcnJheShucCwgImRhdGFfd2lkdGgiLCBh cnIsDQo+ICsJCQkJcGRhdGEtPm5yX21hc3RlcnMpKQ0KPiArCQlmb3IgKGNvdW50ID0gMDsgY291 bnQgPCBwZGF0YS0+bnJfbWFzdGVyczsgY291bnQrKykNCj4gKwkJCXBkYXRhLT5kYXRhX3dpZHRo W2NvdW50XSA9IGFycltjb3VudF07DQpBaCwgaXQgd291bGQgYmUgbmljZSB0byBoYXZlIG9mX3By b3BlcnR5X3JlYWRfdThfYXJyYXkgYW5kIHNvIG9uLi4uDQoNCj4gKw0KPiArCS8qIHBhcnNlIHNs YXZlIGRhdGEgKi8NCj4gKwlzbiA9IG9mX2ZpbmRfbm9kZV9ieV9uYW1lKG5wLCAic2xhdmVfaW5m byIpOw0KPiArCWlmICghc24pDQo+ICsJCXJldHVybiBwZGF0YTsNCj4gKw0KPiArCWNvdW50ID0g MDsNCj4gKwkvKiBjYWxjdWxhdGUgbnVtYmVyIG9mIHNsYXZlcyAqLw0KPiArCWZvcl9lYWNoX2No aWxkX29mX25vZGUoc24sIGNuKQ0KPiArCQljb3VudCsrOw0KDQpvZi5oOiBzdGF0aWMgaW5saW5l IGludCBvZl9nZXRfY2hpbGRfY291bnQoY29uc3Qgc3RydWN0IGRldmljZV9ub2RlICpucCkNCg0K DQo+ICsNCj4gKwlpZiAoIWNvdW50KQ0KPiArCQlyZXR1cm4gTlVMTDsNCj4gKw0KPiArCXNkID0g ZGV2bV9remFsbG9jKCZwZGV2LT5kZXYsIHNpemVvZigqc2QpICogY291bnQsIEdGUF9LRVJORUwp Ow0KPiArCWlmICghc2QpDQo+ICsJCXJldHVybiBOVUxMOw0KPiArDQo+ICsJY291bnQgPSAwOw0K PiArCWZvcl9lYWNoX2NoaWxkX29mX25vZGUoc24sIGNuKSB7DQo+ICsJCW9mX3Byb3BlcnR5X3Jl YWRfc3RyaW5nKGNuLCAiYnVzX2lkIiwgJnNkW2NvdW50XS5idXNfaWQpOw0KPiArCQlvZl9wcm9w ZXJ0eV9yZWFkX3UzMihjbiwgImNmZ19oaSIsICZzZFtjb3VudF0uY2ZnX2hpKTsNCj4gKwkJb2Zf cHJvcGVydHlfcmVhZF91MzIoY24sICJjZmdfbG8iLCAmc2RbY291bnRdLmNmZ19sbyk7DQo+ICsJ CWlmICghb2ZfcHJvcGVydHlfcmVhZF91MzIoY24sICJzcmNfbWFzdGVyIiwgJnZhbCkpDQo+ICsJ CQlzZFtjb3VudF0uc3JjX21hc3RlciA9ICh1OCl2YWw7DQpFeHBsaWNpdCBjYXN0aW5nPw0KDQo+ ICsNCj4gKwkJaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMihjbiwgImRzdF9tYXN0ZXIiLCAmdmFs KSkNCj4gKwkJCXNkW2NvdW50XS5kc3RfbWFzdGVyID0gKHU4KXZhbDsNCmRpdHRvIA0KPiArDQo+ ICsJCXNkW2NvdW50XS5kbWFfZGV2ID0gJnBkZXYtPmRldjsNCj4gKwkJY291bnQrKzsNCj4gKwl9 DQoNCndoYXQgYWJvdXQgdG8gZGVmaW5lIHNkIGFzIHNkWzBdOyBpbiB0aGUgcGxhdGZvcm1fZGF0 YSBzdHJ1Y3R1cmUgYW5kDQp0aGVuIHVzZSBzbXRoIGxpa2UgZm9sbG93aW5nDQoNCnN0cnVjdCAu Li4gKnNkID0gcGRhdGEtPnNkOw0KZm9yX2VhY2guLi4gew0Kc2QtPi4uLiA9IDsNCnNkKys7DQp9 DQoNCml0IHdpbGwgcHJvYmFibHkgcmVxdWlyZSB0byBzcGxpdCB0aGlzIHBhcnQgaW4gYSBzZXBh cmF0ZSBmdW5jdGlvbiBhbmQNCmNhbGN1bGF0ZSBjb3VudCBvZiBzbGF2ZV9pbmZvIGNoaWxkcmVu IGJlZm9yZSBwZGF0YSBtZW1vcnkgYWxsb2NhdGlvbi4NCg0KDQo+ICsNCj4gKwlwZGF0YS0+c2Qg PSBzZDsNCj4gKwlwZGF0YS0+c2RfY291bnQgPSBjb3VudDsNCj4gKw0KPiArCXJldHVybiBwZGF0 YTsNCj4gK30NCg0KLS0gDQpBbmR5IFNoZXZjaGVua28gPGFuZHJpeS5zaGV2Y2hlbmtvQGxpbnV4 LmludGVsLmNvbT4NCkludGVsIEZpbmxhbmQgT3kNCg0KLS0gDQpBbmR5IFNoZXZjaGVua28gPGFu ZHJpeS5zaGV2Y2hlbmtvQGludGVsLmNvbT4NCkludGVsIEZpbmxhbmQgT3kNCi0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LQpJbnRlbCBGaW5sYW5kIE95ClJlZ2lzdGVyZWQgQWRkcmVzczogUEwgMjgxLCAwMDE4MSBIZWxz aW5raSAKQnVzaW5lc3MgSWRlbnRpdHkgQ29kZTogMDM1NzYwNiAtIDQgCkRvbWljaWxlZCBpbiBI ZWxzaW5raSAKClRoaXMgZS1tYWlsIGFuZCBhbnkgYXR0YWNobWVudHMgbWF5IGNvbnRhaW4gY29u ZmlkZW50aWFsIG1hdGVyaWFsIGZvcgp0aGUgc29sZSB1c2Ugb2YgdGhlIGludGVuZGVkIHJlY2lw aWVudChzKS4gQW55IHJldmlldyBvciBkaXN0cmlidXRpb24KYnkgb3RoZXJzIGlzIHN0cmljdGx5 IHByb2hpYml0ZWQuIElmIHlvdSBhcmUgbm90IHRoZSBpbnRlbmRlZApyZWNpcGllbnQsIHBsZWFz ZSBjb250YWN0IHRoZSBzZW5kZXIgYW5kIGRlbGV0ZSBhbGwgY29waWVzLgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759529Ab2JLN25 (ORCPT ); Fri, 12 Oct 2012 09:28:57 -0400 Received: from mga14.intel.com ([143.182.124.37]:18189 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757330Ab2JLN2z (ORCPT ); Fri, 12 Oct 2012 09:28:55 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.80,576,1344236400"; d="scan'208";a="203725931" 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: AQHNqH1/4m+3vxUD3EiqzcH7Q9+iIA== Date: Fri, 12 Oct 2012 13:28:43 +0000 Message-ID: <1350048519.10584.158.camel@smile> References: <142ef9170a2c69657d8a05ac127a9970d7b04965.1350020375.git.viresh.kumar@linaro.org> <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> In-Reply-To: <91e41b4cc972d298f714cbd6f400569a9710304c.1350020375.git.viresh.kumar@linaro.org> 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: <2EB5CA6FD0272640BF09A738D5796869@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 q9CDT2kK015790 On Fri, 2012-10-12 at 11:14 +0530, Viresh Kumar wrote: > dw_dmac driver already supports device tree but it used to have its platform > data passed the non-DT way. Another portion of comments. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > +static struct dw_dma_platform_data * > +__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]; > + > + if (!np) { > + dev_err(&pdev->dev, "Missing DT data\n"); > + return NULL; > + } > + > + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL); > + if (!pdata) > + return NULL; > + > + if (of_property_read_u32(np, "nr_channels", &pdata->nr_channels)) > + return NULL; > + > + if (of_property_read_bool(np, "is_private")) > + pdata->is_private = true; > + > + if (!of_property_read_u32(np, "chan_allocation_order", > + &val)) Fits one line? > + pdata->chan_allocation_order = (unsigned char)val; do we really need explicit casting here? > + > + if (!of_property_read_u32(np, "chan_priority", &val)) > + pdata->chan_priority = (unsigned char)val; ditto > + > + if (!of_property_read_u32(np, "block_size", &val)) > + pdata->block_size = (unsigned short)val; ditto > + > + 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. > + return NULL; > + > + pdata->nr_masters = (unsigned char)val; > + } > + > + 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... > + > + /* parse slave data */ > + sn = of_find_node_by_name(np, "slave_info"); > + if (!sn) > + return pdata; > + > + count = 0; > + /* calculate number of slaves */ > + for_each_child_of_node(sn, cn) > + count++; of.h: static inline int of_get_child_count(const struct device_node *np) > + > + if (!count) > + return NULL; > + > + sd = devm_kzalloc(&pdev->dev, sizeof(*sd) * count, GFP_KERNEL); > + if (!sd) > + return NULL; > + > + count = 0; > + 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); > + if (!of_property_read_u32(cn, "src_master", &val)) > + sd[count].src_master = (u8)val; Explicit casting? > + > + if (!of_property_read_u32(cn, "dst_master", &val)) > + sd[count].dst_master = (u8)val; ditto > + > + sd[count].dma_dev = &pdev->dev; > + count++; > + } what about to define sd as sd[0]; in the platform_data structure and then use smth like following struct ... *sd = pdata->sd; for_each... { sd->... = ; sd++; } it will probably require to split this part in a separate function and calculate count of slave_info children before pdata memory allocation. > + > + pdata->sd = sd; > + pdata->sd_count = count; > + > + return pdata; > +} -- Andy Shevchenko Intel Finland Oy -- 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