From mboxrd@z Thu Jan 1 00:00:00 1970 From: b32955@freescale.com (Huang Shijie) Date: Thu, 23 May 2013 17:35:42 +0800 Subject: [PATCH V2 1/6] drivers: bus: add a new driver for WEIM In-Reply-To: <20130523092344.GF32299@pengutronix.de> References: <1369296978-7669-1-git-send-email-b32955@freescale.com> <1369296978-7669-2-git-send-email-b32955@freescale.com> <20130523092344.GF32299@pengutronix.de> Message-ID: <519DE2EE.1050006@freescale.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org ? 2013?05?23? 17:23, Sascha Hauer ??: > On Thu, May 23, 2013 at 04:16:13PM +0800, Huang Shijie wrote: >> + 0 >> + >> +Timing property for child nodes. It is mandatory, not optional. >> + >> + - fsl,weim-cs-timing: The timing array, contains 6 timing values for the >> + child node. We can get the CS index from the child >> + node's "reg" property. > This should be more detailed, something like: > > This contains the values for the registers EIM_CSnGCR1, EIM_CSnGCR2, > EIM_CSnRCR1, EIM_CSnRCR2, EIM_CSnWCR1, EIM_CSnWCR2 in this order. > do you mean i should add some new properties, such as "fsl,eim_csnrcr1", "fsl,eim_csnrcr2" ... >> +static int weim_parse_dt(struct platform_device *pdev) >> +{ >> + struct device_node *child; >> + int ret; >> + >> + for_each_child_of_node(pdev->dev.of_node, child) { >> + if (!child->name) >> + continue; >> + >> + ret = weim_timing_setup(pdev, child); >> + if (ret) >> + goto parse_fail; >> + >> + if (!of_platform_device_create(child, NULL,&pdev->dev)) { >> + ret = -EINVAL; >> + goto parse_fail; >> + } > I would print some warning here for the failures in this loop, but I > don't think it's necessary to bail out. No need to make all WEIM devices > fail if one has an erroneous device node. > ok. >> + >> + /* get the resource */ >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + weim->base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(weim->base)) { >> + ret = PTR_ERR(weim->base); >> + goto weim_err; >> + } >> + >> + /* get the clock */ >> + weim->clk = devm_clk_get(&pdev->dev, NULL); >> + if (IS_ERR(weim->clk)) >> + goto weim_err; >> + >> + clk_prepare_enable(weim->clk); >> + >> + /* parse the device node */ >> + ret = weim_parse_dt(pdev); >> + if (ret) { >> + clk_disable_unprepare(weim->clk); >> + goto weim_err; >> + } >> + >> + dev_info(&pdev->dev, "WEIM driver registered.\n"); >> + return 0; >> + >> +weim_err: >> + return ret; >> +} >> + >> +static int weim_remove(struct platform_device *pdev) >> +{ >> + struct imx_weim *weim = platform_get_drvdata(pdev); >> + >> + clk_disable_unprepare(weim->clk); > Once again: Is this clock needed for the child devices? If yes, you > can't disable it here and leave the child devices registered. > again. yes. we need this clock. thanks Huang Shijie From mboxrd@z Thu Jan 1 00:00:00 1970 From: Huang Shijie Subject: Re: [PATCH V2 1/6] drivers: bus: add a new driver for WEIM Date: Thu, 23 May 2013 17:35:42 +0800 Message-ID: <519DE2EE.1050006@freescale.com> References: <1369296978-7669-1-git-send-email-b32955@freescale.com> <1369296978-7669-2-git-send-email-b32955@freescale.com> <20130523092344.GF32299@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20130523092344.GF32299-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Sascha Hauer Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Alison_Chaiken-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org 5LqOIDIwMTPlubQwNeaciDIz5pelIDE3OjIzLCBTYXNjaGEgSGF1ZXIg5YaZ6YGTOgo+IE9uIFRo dSwgTWF5IDIzLCAyMDEzIGF0IDA0OjE2OjEzUE0gKzA4MDAsIEh1YW5nIFNoaWppZSB3cm90ZToK Pj4gKwkJCTxjcy1udW1iZXI+ICAwPHBoeXNpY2FsIGFkZHJlc3Mgb2YgbWFwcGluZz4gIDxzaXpl Pgo+PiArCj4+ICtUaW1pbmcgcHJvcGVydHkgZm9yIGNoaWxkIG5vZGVzLiBJdCBpcyBtYW5kYXRv cnksIG5vdCBvcHRpb25hbC4KPj4gKwo+PiArIC0gZnNsLHdlaW0tY3MtdGltaW5nOglUaGUgdGlt aW5nIGFycmF5LCBjb250YWlucyA2IHRpbWluZyB2YWx1ZXMgZm9yIHRoZQo+PiArCQkJY2hpbGQg bm9kZS4gV2UgY2FuIGdldCB0aGUgQ1MgaW5kZXggZnJvbSB0aGUgY2hpbGQKPj4gKwkJCW5vZGUn cyAicmVnIiBwcm9wZXJ0eS4KPiBUaGlzIHNob3VsZCBiZSBtb3JlIGRldGFpbGVkLCBzb21ldGhp bmcgbGlrZToKPgo+IFRoaXMgY29udGFpbnMgdGhlIHZhbHVlcyBmb3IgdGhlIHJlZ2lzdGVycyBF SU1fQ1NuR0NSMSwgRUlNX0NTbkdDUjIsCj4gRUlNX0NTblJDUjEsIEVJTV9DU25SQ1IyLCBFSU1f Q1NuV0NSMSwgRUlNX0NTbldDUjIgaW4gdGhpcyBvcmRlci4KPgoKCmRvIHlvdSBtZWFuIGkgc2hv dWxkIGFkZCBzb21lIG5ldyBwcm9wZXJ0aWVzLCBzdWNoIGFzCiJmc2wsZWltX2NzbnJjcjEiLCAi ZnNsLGVpbV9jc25yY3IyIiAuLi4KPj4gK3N0YXRpYyBpbnQgd2VpbV9wYXJzZV9kdChzdHJ1Y3Qg cGxhdGZvcm1fZGV2aWNlICpwZGV2KQo+PiArewo+PiArCXN0cnVjdCBkZXZpY2Vfbm9kZSAqY2hp bGQ7Cj4+ICsJaW50IHJldDsKPj4gKwo+PiArCWZvcl9lYWNoX2NoaWxkX29mX25vZGUocGRldi0+ ZGV2Lm9mX25vZGUsIGNoaWxkKSB7Cj4+ICsJCWlmICghY2hpbGQtPm5hbWUpCj4+ICsJCQljb250 aW51ZTsKPj4gKwo+PiArCQlyZXQgPSB3ZWltX3RpbWluZ19zZXR1cChwZGV2LCBjaGlsZCk7Cj4+ ICsJCWlmIChyZXQpCj4+ICsJCQlnb3RvIHBhcnNlX2ZhaWw7Cj4+ICsKPj4gKwkJaWYgKCFvZl9w bGF0Zm9ybV9kZXZpY2VfY3JlYXRlKGNoaWxkLCBOVUxMLCZwZGV2LT5kZXYpKSB7Cj4+ICsJCQly ZXQgPSAtRUlOVkFMOwo+PiArCQkJZ290byBwYXJzZV9mYWlsOwo+PiArCQl9Cj4gSSB3b3VsZCBw cmludCBzb21lIHdhcm5pbmcgaGVyZSBmb3IgdGhlIGZhaWx1cmVzIGluIHRoaXMgbG9vcCwgYnV0 IEkKPiBkb24ndCB0aGluayBpdCdzIG5lY2Vzc2FyeSB0byBiYWlsIG91dC4gTm8gbmVlZCB0byBt YWtlIGFsbCBXRUlNIGRldmljZXMKPiBmYWlsIGlmIG9uZSBoYXMgYW4gZXJyb25lb3VzIGRldmlj ZSBub2RlLgo+Cm9rLgo+PiArCj4+ICsJLyogZ2V0IHRoZSByZXNvdXJjZSAqLwo+PiArCXJlcyA9 IHBsYXRmb3JtX2dldF9yZXNvdXJjZShwZGV2LCBJT1JFU09VUkNFX01FTSwgMCk7Cj4+ICsJd2Vp bS0+YmFzZSA9IGRldm1faW9yZW1hcF9yZXNvdXJjZSgmcGRldi0+ZGV2LCByZXMpOwo+PiArCWlm IChJU19FUlIod2VpbS0+YmFzZSkpIHsKPj4gKwkJcmV0ID0gUFRSX0VSUih3ZWltLT5iYXNlKTsK Pj4gKwkJZ290byB3ZWltX2VycjsKPj4gKwl9Cj4+ICsKPj4gKwkvKiBnZXQgdGhlIGNsb2NrICov Cj4+ICsJd2VpbS0+Y2xrID0gZGV2bV9jbGtfZ2V0KCZwZGV2LT5kZXYsIE5VTEwpOwo+PiArCWlm IChJU19FUlIod2VpbS0+Y2xrKSkKPj4gKwkJZ290byB3ZWltX2VycjsKPj4gKwo+PiArCWNsa19w cmVwYXJlX2VuYWJsZSh3ZWltLT5jbGspOwo+PiArCj4+ICsJLyogcGFyc2UgdGhlIGRldmljZSBu b2RlICovCj4+ICsJcmV0ID0gd2VpbV9wYXJzZV9kdChwZGV2KTsKPj4gKwlpZiAocmV0KSB7Cj4+ ICsJCWNsa19kaXNhYmxlX3VucHJlcGFyZSh3ZWltLT5jbGspOwo+PiArCQlnb3RvIHdlaW1fZXJy Owo+PiArCX0KPj4gKwo+PiArCWRldl9pbmZvKCZwZGV2LT5kZXYsICJXRUlNIGRyaXZlciByZWdp c3RlcmVkLlxuIik7Cj4+ICsJcmV0dXJuIDA7Cj4+ICsKPj4gK3dlaW1fZXJyOgo+PiArCXJldHVy biByZXQ7Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyBpbnQgd2VpbV9yZW1vdmUoc3RydWN0IHBsYXRm b3JtX2RldmljZSAqcGRldikKPj4gK3sKPj4gKwlzdHJ1Y3QgaW14X3dlaW0gKndlaW0gPSBwbGF0 Zm9ybV9nZXRfZHJ2ZGF0YShwZGV2KTsKPj4gKwo+PiArCWNsa19kaXNhYmxlX3VucHJlcGFyZSh3 ZWltLT5jbGspOwo+IE9uY2UgYWdhaW46IElzIHRoaXMgY2xvY2sgbmVlZGVkIGZvciB0aGUgY2hp bGQgZGV2aWNlcz8gSWYgeWVzLCB5b3UKPiBjYW4ndCBkaXNhYmxlIGl0IGhlcmUgYW5kIGxlYXZl IHRoZSBjaGlsZCBkZXZpY2VzIHJlZ2lzdGVyZWQuCj4KYWdhaW4uIHllcy4gd2UgbmVlZCB0aGlz IGNsb2NrLgoKCnRoYW5rcwpIdWFuZyBTaGlqaWUKCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCmRldmljZXRyZWUtZGlzY3VzcyBtYWlsaW5nIGxpc3QKZGV2 aWNldHJlZS1kaXNjdXNzQGxpc3RzLm96bGFicy5vcmcKaHR0cHM6Ly9saXN0cy5vemxhYnMub3Jn L2xpc3RpbmZvL2RldmljZXRyZWUtZGlzY3Vzcwo=