From mboxrd@z Thu Jan 1 00:00:00 1970 From: b32955@freescale.com (Huang Shijie) Date: Wed, 22 May 2013 16:30:03 +0800 Subject: [PATCH 1/6] drivers: bus: add a new driver for WEIM In-Reply-To: <20130520131827.GB32299@pengutronix.de> References: <1369039742-10893-1-git-send-email-b32955@freescale.com> <1369039742-10893-2-git-send-email-b32955@freescale.com> <20130520131827.GB32299@pengutronix.de> Message-ID: <519C820B.2080800@freescale.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org ? 2013?05?20? 21:18, Sascha Hauer ??: > On Mon, May 20, 2013 at 04:48:57PM +0800, Huang Shijie wrote: >> weim-cs-index. >> + >> +Example for an i.MX6q-sabreauto board: >> + weim: weim at 021b8000 { >> + compatible = "fsl,imx6q-weim"; >> + reg =<0x021b8000 0x4000>; >> + interrupts =<0 14 0x04>; >> + clocks =<&clks 196>; >> + #address-cells =<2>; > Why is address cells 2 in this example? Must be set to 2 to allow memory address translation. >> + #size-cells =<1>; >> + ranges =<0 0 0x08000000 0x08000000>; >> + >> + nor at 0,0 { >> + compatible = "cfi-flash"; >> + reg =<0 0 0x02000000>; >> + #address-cells =<1>; >> + #size-cells =<1>; >> + bank-width =<2>; >> + >> + weim-cs-index =<0>; >> + weim-cs-timing =<0x00620081 0x00000001 0x1C022000 >> + 0x0000C000 0x1404a38e 0x00000000>; >> + partition at 0 { >> + label = "U-Boot"; >> + reg =<0x0 0x40000>; >> + }; >> + >> + partition at 40000 { >> + label = "U-Boot-ENV"; >> + reg =<0x40000 0x10000>; >> + read-only; >> + }; >> + >> + partition at 50000 { >> + label = "Kernel"; >> + reg =<0x50000 0x3b0000>; >> + }; > The partitions are unnecessary to understand the example. Please drop them. okay. >> +#define CS_TIMING_LEN 6 >> +#define CS_REG_RANGE 0x18 >> +/* Parse and set the timing for this device. */ >> +static int weim_timing(struct platform_device *pdev, struct device_node *np) >> +{ >> + struct imx_weim *weim = platform_get_drvdata(pdev); >> + u32 value[CS_TIMING_LEN]; >> + u32 cs_idx; >> + int ret; >> + int i; >> + >> + ret = of_property_read_u32(np, "weim-cs-index",&cs_idx); >> + if (ret) >> + goto weim_parse_err; > It would be nice to check for cs_idx being valid. > >> + >> + ret = of_property_read_u32_array(np, "weim-cs-timing", >> + value, CS_TIMING_LEN); >> + if (ret) >> + goto weim_parse_err; >> + >> + /* set the timing for WEIM */ >> + for (i = 0; i< CS_TIMING_LEN; i++) >> + writel(value[i], weim->base + cs_idx * CS_REG_RANGE + i * 4); >> + return 0; >> + >> +weim_parse_err: >> + return ret; >> +} >> + >> +static int weim_parse_dt(struct platform_device *pdev) >> +{ >> + struct device_node *child; >> + int ret; >> + >> + /* We only support the Parallel NOR now. We may add more in future. */ >> + child = of_find_node_by_name(NULL, "nor"); >> + if (child) { >> + ret = weim_timing(pdev, child); >> + if (ret) >> + goto parse_fail; >> + >> + if (!of_platform_device_create(child, NULL,&pdev->dev)) { >> + ret = -EINVAL; >> + goto parse_fail; >> + } >> + } > What you want to do here is: > > - iterate over your child nodes > - call weim_timing() for each of them > - call of_platform_device_create for each child (or even > of_platform_populate() with the parent node) > yes. >> + return 0; >> + >> +parse_fail: >> + return ret; >> +} >> + >> +static int weim_probe(struct platform_device *pdev) >> +{ >> + struct imx_weim *weim; >> + struct resource *res; >> + int ret = -EINVAL; >> + >> + weim = devm_kzalloc(&pdev->dev, sizeof(*weim), GFP_KERNEL); >> + if (!weim) { >> + ret = -ENOMEM; >> + goto weim_err; >> + } >> + platform_set_drvdata(pdev, weim); >> + >> + /* get the resource */ >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!res) { >> + ret = -ENOENT; >> + goto weim_err; >> + } > No need to do error checking here. devm_ioremap_resource() will do this > for you and also print an error message. > >> + >> + 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); > what is this clock used for? Is it necessary for the register access or > is it necessary for the chipselects on the WEIM to work? > > yes. We need this clock. in actually, this clock is just a clock gate for several clocks, including clock for register access, and other necessary clocks. thanks Huang Shijie . From mboxrd@z Thu Jan 1 00:00:00 1970 From: Huang Shijie Subject: Re: [PATCH 1/6] drivers: bus: add a new driver for WEIM Date: Wed, 22 May 2013 16:30:03 +0800 Message-ID: <519C820B.2080800@freescale.com> References: <1369039742-10893-1-git-send-email-b32955@freescale.com> <1369039742-10893-2-git-send-email-b32955@freescale.com> <20130520131827.GB32299@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20130520131827.GB32299-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: grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org 5LqOIDIwMTPlubQwNeaciDIw5pelIDIxOjE4LCBTYXNjaGEgSGF1ZXIg5YaZ6YGTOgo+IE9uIE1v biwgTWF5IDIwLCAyMDEzIGF0IDA0OjQ4OjU3UE0gKzA4MDAsIEh1YW5nIFNoaWppZSB3cm90ZToK Pj4gCQkJd2VpbS1jcy1pbmRleC4KPj4gKwo+PiArRXhhbXBsZSBmb3IgYW4gaS5NWDZxLXNhYnJl YXV0byBib2FyZDoKPj4gKwl3ZWltOiB3ZWltQDAyMWI4MDAwIHsKPj4gKwkJY29tcGF0aWJsZSA9 ICJmc2wsaW14NnEtd2VpbSI7Cj4+ICsJCXJlZyA9PDB4MDIxYjgwMDAgMHg0MDAwPjsKPj4gKwkJ aW50ZXJydXB0cyA9PDAgMTQgMHgwND47Cj4+ICsJCWNsb2NrcyA9PCZjbGtzIDE5Nj47Cj4+ICsJ CSNhZGRyZXNzLWNlbGxzID08Mj47Cj4gV2h5IGlzIGFkZHJlc3MgY2VsbHMgMiBpbiB0aGlzIGV4 YW1wbGU/Ck11c3QgYmUgc2V0IHRvIDIgdG8gYWxsb3cgbWVtb3J5IGFkZHJlc3MgdHJhbnNsYXRp b24uCj4+ICsJCSNzaXplLWNlbGxzID08MT47Cj4+ICsJCXJhbmdlcyA9PDAgMCAweDA4MDAwMDAw IDB4MDgwMDAwMDA+Owo+PiArCj4+ICsJCW5vckAwLDAgewo+PiArCQkJY29tcGF0aWJsZSA9ICJj ZmktZmxhc2giOwo+PiArCQkJcmVnID08MCAwIDB4MDIwMDAwMDA+Owo+PiArCQkJI2FkZHJlc3Mt Y2VsbHMgPTwxPjsKPj4gKwkJCSNzaXplLWNlbGxzID08MT47Cj4+ICsJCQliYW5rLXdpZHRoID08 Mj47Cj4+ICsKPj4gKwkJCXdlaW0tY3MtaW5kZXggPTwwPjsKPj4gKwkJCXdlaW0tY3MtdGltaW5n ID08MHgwMDYyMDA4MSAweDAwMDAwMDAxIDB4MUMwMjIwMDAKPj4gKwkJCQkJMHgwMDAwQzAwMCAw eDE0MDRhMzhlIDB4MDAwMDAwMDA+Owo+PiArCQkJcGFydGl0aW9uQDAgewo+PiArCQkJCWxhYmVs ID0gIlUtQm9vdCI7Cj4+ICsJCQkJcmVnID08MHgwIDB4NDAwMDA+Owo+PiArCQkJfTsKPj4gKwo+ PiArCQkJcGFydGl0aW9uQDQwMDAwIHsKPj4gKwkJCQlsYWJlbCA9ICJVLUJvb3QtRU5WIjsKPj4g KwkJCQlyZWcgPTwweDQwMDAwIDB4MTAwMDA+Owo+PiArCQkJCXJlYWQtb25seTsKPj4gKwkJCX07 Cj4+ICsKPj4gKwkJCXBhcnRpdGlvbkA1MDAwMCB7Cj4+ICsJCQkJbGFiZWwgPSAiS2VybmVsIjsK Pj4gKwkJCQlyZWcgPTwweDUwMDAwIDB4M2IwMDAwPjsKPj4gKwkJCX07Cj4gVGhlIHBhcnRpdGlv bnMgYXJlIHVubmVjZXNzYXJ5IHRvIHVuZGVyc3RhbmQgdGhlIGV4YW1wbGUuIFBsZWFzZSBkcm9w IHRoZW0uCm9rYXkuCj4+ICsjZGVmaW5lIENTX1RJTUlOR19MRU4gNgo+PiArI2RlZmluZSBDU19S RUdfUkFOR0UgIDB4MTgKPj4gKy8qIFBhcnNlIGFuZCBzZXQgdGhlIHRpbWluZyBmb3IgdGhpcyBk ZXZpY2UuICovCj4+ICtzdGF0aWMgaW50IHdlaW1fdGltaW5nKHN0cnVjdCBwbGF0Zm9ybV9kZXZp Y2UgKnBkZXYsIHN0cnVjdCBkZXZpY2Vfbm9kZSAqbnApCj4+ICt7Cj4+ICsJc3RydWN0IGlteF93 ZWltICp3ZWltID0gcGxhdGZvcm1fZ2V0X2RydmRhdGEocGRldik7Cj4+ICsJdTMyIHZhbHVlW0NT X1RJTUlOR19MRU5dOwo+PiArCXUzMiBjc19pZHg7Cj4+ICsJaW50IHJldDsKPj4gKwlpbnQgaTsK Pj4gKwo+PiArCXJldCA9IG9mX3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAid2VpbS1jcy1pbmRleCIs JmNzX2lkeCk7Cj4+ICsJaWYgKHJldCkKPj4gKwkJZ290byB3ZWltX3BhcnNlX2VycjsKPiBJdCB3 b3VsZCBiZSBuaWNlIHRvIGNoZWNrIGZvciBjc19pZHggYmVpbmcgdmFsaWQuCj4KPj4gKwo+PiAr CXJldCA9IG9mX3Byb3BlcnR5X3JlYWRfdTMyX2FycmF5KG5wLCAid2VpbS1jcy10aW1pbmciLAo+ PiArCQkJCQl2YWx1ZSwgQ1NfVElNSU5HX0xFTik7Cj4+ICsJaWYgKHJldCkKPj4gKwkJZ290byB3 ZWltX3BhcnNlX2VycjsKPj4gKwo+PiArCS8qIHNldCB0aGUgdGltaW5nIGZvciBXRUlNICovCj4+ ICsJZm9yIChpID0gMDsgaTwgIENTX1RJTUlOR19MRU47IGkrKykKPj4gKwkJd3JpdGVsKHZhbHVl W2ldLCB3ZWltLT5iYXNlICsgY3NfaWR4ICogQ1NfUkVHX1JBTkdFICsgaSAqIDQpOwo+PiArCXJl dHVybiAwOwo+PiArCj4+ICt3ZWltX3BhcnNlX2VycjoKPj4gKwlyZXR1cm4gcmV0Owo+PiArfQo+ PiArCj4+ICtzdGF0aWMgaW50IHdlaW1fcGFyc2VfZHQoc3RydWN0IHBsYXRmb3JtX2RldmljZSAq cGRldikKPj4gK3sKPj4gKwlzdHJ1Y3QgZGV2aWNlX25vZGUgKmNoaWxkOwo+PiArCWludCByZXQ7 Cj4+ICsKPj4gKwkvKiBXZSBvbmx5IHN1cHBvcnQgdGhlIFBhcmFsbGVsIE5PUiBub3cuIFdlIG1h eSBhZGQgbW9yZSBpbiBmdXR1cmUuICovCj4+ICsJY2hpbGQgPSBvZl9maW5kX25vZGVfYnlfbmFt ZShOVUxMLCAibm9yIik7Cj4+ICsJaWYgKGNoaWxkKSB7Cj4+ICsJCXJldCA9IHdlaW1fdGltaW5n KHBkZXYsIGNoaWxkKTsKPj4gKwkJaWYgKHJldCkKPj4gKwkJCWdvdG8gcGFyc2VfZmFpbDsKPj4g Kwo+PiArCQlpZiAoIW9mX3BsYXRmb3JtX2RldmljZV9jcmVhdGUoY2hpbGQsIE5VTEwsJnBkZXYt PmRldikpIHsKPj4gKwkJCXJldCA9IC1FSU5WQUw7Cj4+ICsJCQlnb3RvIHBhcnNlX2ZhaWw7Cj4+ ICsJCX0KPj4gKwl9Cj4gV2hhdCB5b3Ugd2FudCB0byBkbyBoZXJlIGlzOgo+Cj4gLSBpdGVyYXRl IG92ZXIgeW91ciBjaGlsZCBub2Rlcwo+IC0gY2FsbCB3ZWltX3RpbWluZygpIGZvciBlYWNoIG9m IHRoZW0KPiAtIGNhbGwgb2ZfcGxhdGZvcm1fZGV2aWNlX2NyZWF0ZSBmb3IgZWFjaCBjaGlsZCAo b3IgZXZlbgo+ICAgIG9mX3BsYXRmb3JtX3BvcHVsYXRlKCkgd2l0aCB0aGUgcGFyZW50IG5vZGUp Cj4KeWVzLgo+PiArCXJldHVybiAwOwo+PiArCj4+ICtwYXJzZV9mYWlsOgo+PiArCXJldHVybiBy ZXQ7Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyBpbnQgd2VpbV9wcm9iZShzdHJ1Y3QgcGxhdGZvcm1f ZGV2aWNlICpwZGV2KQo+PiArewo+PiArCXN0cnVjdCBpbXhfd2VpbSAqd2VpbTsKPj4gKwlzdHJ1 Y3QgcmVzb3VyY2UgKnJlczsKPj4gKwlpbnQgcmV0ID0gLUVJTlZBTDsKPj4gKwo+PiArCXdlaW0g PSBkZXZtX2t6YWxsb2MoJnBkZXYtPmRldiwgc2l6ZW9mKCp3ZWltKSwgR0ZQX0tFUk5FTCk7Cj4+ ICsJaWYgKCF3ZWltKSB7Cj4+ICsJCXJldCA9IC1FTk9NRU07Cj4+ICsJCWdvdG8gd2VpbV9lcnI7 Cj4+ICsJfQo+PiArCXBsYXRmb3JtX3NldF9kcnZkYXRhKHBkZXYsIHdlaW0pOwo+PiArCj4+ICsJ LyogZ2V0IHRoZSByZXNvdXJjZSAqLwo+PiArCXJlcyA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZShw ZGV2LCBJT1JFU09VUkNFX01FTSwgMCk7Cj4+ICsJaWYgKCFyZXMpIHsKPj4gKwkJcmV0ID0gLUVO T0VOVDsKPj4gKwkJZ290byB3ZWltX2VycjsKPj4gKwl9Cj4gTm8gbmVlZCB0byBkbyBlcnJvciBj aGVja2luZyBoZXJlLiBkZXZtX2lvcmVtYXBfcmVzb3VyY2UoKSB3aWxsIGRvIHRoaXMKPiBmb3Ig eW91IGFuZCBhbHNvIHByaW50IGFuIGVycm9yIG1lc3NhZ2UuCj4KPj4gKwo+PiArCXdlaW0tPmJh c2UgPSBkZXZtX2lvcmVtYXBfcmVzb3VyY2UoJnBkZXYtPmRldiwgcmVzKTsKPj4gKwlpZiAoSVNf RVJSKHdlaW0tPmJhc2UpKSB7Cj4+ICsJCXJldCA9IFBUUl9FUlIod2VpbS0+YmFzZSk7Cj4+ICsJ CWdvdG8gd2VpbV9lcnI7Cj4+ICsJfQo+PiArCj4+ICsJLyogZ2V0IHRoZSBjbG9jayAqLwo+PiAr CXdlaW0tPmNsayA9IGRldm1fY2xrX2dldCgmcGRldi0+ZGV2LCBOVUxMKTsKPj4gKwlpZiAoSVNf RVJSKHdlaW0tPmNsaykpCj4+ICsJCWdvdG8gd2VpbV9lcnI7Cj4+ICsKPj4gKwljbGtfcHJlcGFy ZV9lbmFibGUod2VpbS0+Y2xrKTsKPiB3aGF0IGlzIHRoaXMgY2xvY2sgdXNlZCBmb3I/IElzIGl0 IG5lY2Vzc2FyeSBmb3IgdGhlIHJlZ2lzdGVyIGFjY2VzcyBvcgo+IGlzIGl0IG5lY2Vzc2FyeSBm b3IgdGhlIGNoaXBzZWxlY3RzIG9uIHRoZSBXRUlNIHRvIHdvcms/Cj4KPgp5ZXMuIFdlIG5lZWQg dGhpcyBjbG9jay4KCmluIGFjdHVhbGx5LCB0aGlzIGNsb2NrIGlzIGp1c3QgYSBjbG9jayBnYXRl IGZvciBzZXZlcmFsIGNsb2NrcywgCmluY2x1ZGluZyBjbG9jayBmb3IgcmVnaXN0ZXIgYWNjZXNz LCBhbmQKb3RoZXIgbmVjZXNzYXJ5IGNsb2Nrcy4KCnRoYW5rcwpIdWFuZyBTaGlqaWUKLgoKCgoK CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkZXZpY2V0 cmVlLWRpc2N1c3MgbWFpbGluZyBsaXN0CmRldmljZXRyZWUtZGlzY3Vzc0BsaXN0cy5vemxhYnMu b3JnCmh0dHBzOi8vbGlzdHMub3psYWJzLm9yZy9saXN0aW5mby9kZXZpY2V0cmVlLWRpc2N1c3MK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754214Ab3EVI1o (ORCPT ); Wed, 22 May 2013 04:27:44 -0400 Received: from va3ehsobe002.messaging.microsoft.com ([216.32.180.12]:16831 "EHLO va3outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750931Ab3EVI1i convert rfc822-to-8bit (ORCPT ); Wed, 22 May 2013 04:27:38 -0400 X-Forefront-Antispam-Report: CIP:70.37.183.190;KIP:(null);UIP:(null);IPV:NLI;H:mail.freescale.net;RD:none;EFVD:NLI X-SpamScore: 16 X-BigFish: VS16(zz98dIc89bh1432Izz1f42h1ee6h1de0h1fdah1202h1e76h1d1ah1d2ah1fc6h1082kzzz2dh2a8h668h839h93fhd25he5bhf0ah1288h12a5h12a9h12bdh1354h137ah13b6h1441h1504h1537h153bh162dh1631h1758h1765h18e1h190ch1946h19c3h1ad9h1b0ah1d0ch1d2eh1d3fh133w1155h) Message-ID: <519C820B.2080800@freescale.com> Date: Wed, 22 May 2013 16:30:03 +0800 From: Huang Shijie User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.24) Gecko/20111108 Fedora/3.1.16-1.fc14 Thunderbird/3.1.16 MIME-Version: 1.0 To: Sascha Hauer CC: , , , , Subject: Re: [PATCH 1/6] drivers: bus: add a new driver for WEIM References: <1369039742-10893-1-git-send-email-b32955@freescale.com> <1369039742-10893-2-git-send-email-b32955@freescale.com> <20130520131827.GB32299@pengutronix.de> In-Reply-To: <20130520131827.GB32299@pengutronix.de> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8BIT X-OriginatorOrg: freescale.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 于 2013年05月20日 21:18, Sascha Hauer 写道: > On Mon, May 20, 2013 at 04:48:57PM +0800, Huang Shijie wrote: >> weim-cs-index. >> + >> +Example for an i.MX6q-sabreauto board: >> + weim: weim@021b8000 { >> + compatible = "fsl,imx6q-weim"; >> + reg =<0x021b8000 0x4000>; >> + interrupts =<0 14 0x04>; >> + clocks =<&clks 196>; >> + #address-cells =<2>; > Why is address cells 2 in this example? Must be set to 2 to allow memory address translation. >> + #size-cells =<1>; >> + ranges =<0 0 0x08000000 0x08000000>; >> + >> + nor@0,0 { >> + compatible = "cfi-flash"; >> + reg =<0 0 0x02000000>; >> + #address-cells =<1>; >> + #size-cells =<1>; >> + bank-width =<2>; >> + >> + weim-cs-index =<0>; >> + weim-cs-timing =<0x00620081 0x00000001 0x1C022000 >> + 0x0000C000 0x1404a38e 0x00000000>; >> + partition@0 { >> + label = "U-Boot"; >> + reg =<0x0 0x40000>; >> + }; >> + >> + partition@40000 { >> + label = "U-Boot-ENV"; >> + reg =<0x40000 0x10000>; >> + read-only; >> + }; >> + >> + partition@50000 { >> + label = "Kernel"; >> + reg =<0x50000 0x3b0000>; >> + }; > The partitions are unnecessary to understand the example. Please drop them. okay. >> +#define CS_TIMING_LEN 6 >> +#define CS_REG_RANGE 0x18 >> +/* Parse and set the timing for this device. */ >> +static int weim_timing(struct platform_device *pdev, struct device_node *np) >> +{ >> + struct imx_weim *weim = platform_get_drvdata(pdev); >> + u32 value[CS_TIMING_LEN]; >> + u32 cs_idx; >> + int ret; >> + int i; >> + >> + ret = of_property_read_u32(np, "weim-cs-index",&cs_idx); >> + if (ret) >> + goto weim_parse_err; > It would be nice to check for cs_idx being valid. > >> + >> + ret = of_property_read_u32_array(np, "weim-cs-timing", >> + value, CS_TIMING_LEN); >> + if (ret) >> + goto weim_parse_err; >> + >> + /* set the timing for WEIM */ >> + for (i = 0; i< CS_TIMING_LEN; i++) >> + writel(value[i], weim->base + cs_idx * CS_REG_RANGE + i * 4); >> + return 0; >> + >> +weim_parse_err: >> + return ret; >> +} >> + >> +static int weim_parse_dt(struct platform_device *pdev) >> +{ >> + struct device_node *child; >> + int ret; >> + >> + /* We only support the Parallel NOR now. We may add more in future. */ >> + child = of_find_node_by_name(NULL, "nor"); >> + if (child) { >> + ret = weim_timing(pdev, child); >> + if (ret) >> + goto parse_fail; >> + >> + if (!of_platform_device_create(child, NULL,&pdev->dev)) { >> + ret = -EINVAL; >> + goto parse_fail; >> + } >> + } > What you want to do here is: > > - iterate over your child nodes > - call weim_timing() for each of them > - call of_platform_device_create for each child (or even > of_platform_populate() with the parent node) > yes. >> + return 0; >> + >> +parse_fail: >> + return ret; >> +} >> + >> +static int weim_probe(struct platform_device *pdev) >> +{ >> + struct imx_weim *weim; >> + struct resource *res; >> + int ret = -EINVAL; >> + >> + weim = devm_kzalloc(&pdev->dev, sizeof(*weim), GFP_KERNEL); >> + if (!weim) { >> + ret = -ENOMEM; >> + goto weim_err; >> + } >> + platform_set_drvdata(pdev, weim); >> + >> + /* get the resource */ >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!res) { >> + ret = -ENOENT; >> + goto weim_err; >> + } > No need to do error checking here. devm_ioremap_resource() will do this > for you and also print an error message. > >> + >> + 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); > what is this clock used for? Is it necessary for the register access or > is it necessary for the chipselects on the WEIM to work? > > yes. We need this clock. in actually, this clock is just a clock gate for several clocks, including clock for register access, and other necessary clocks. thanks Huang Shijie .