From: Barry Song <21cnbao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> To: Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> Cc: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org, John Bonesio <bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>, linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, olofj-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Subject: Re: [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree. Date: Fri, 3 Jun 2011 10:32:52 +0800 [thread overview] Message-ID: <BANLkTikiaA3CCRAXkB=x56wtt8nNF9dqxA@mail.gmail.com> (raw) In-Reply-To: <20110602214322.GC10532-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2011/6/3 Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>: > On Fri, Jun 03, 2011 at 12:21:50AM +0800, Barry Song wrote: >> Arnd has required me to use device tree in our new SoC for the coming >> upstream. so i am trying to define a property like clock = "uart" in >> dts. then in drivers, >> i get this string by: >> clk = of_get_property(np, "clock", NULL); >> then request this clock by clk_get(). > > This is entirely wrong. clk_get() takes two things. It takes a struct > device. We should know what the struct device is (provided they're named > in a stable manner.) > > The other parameter, the string, is up to the driver. It's not a device > property. It's not a SoC-wide clock name. It's a connection name for > the clock on the device. This won't change from one instance of the > device to another instance of the device - it's effectively a constant. > i see :-) but there is really no an unified rule by now, for exmaple, samsung just required platform device names matched with the string parameter to get a clock. it looks like clk_get in plat-samsung depends on the string more than device and the clock name is in SoC level. struct clk *clk_get(struct device *dev, const char *id) { struct clk *p; struct clk *clk = ERR_PTR(-ENOENT); int idno; if (dev == NULL || !dev_is_platform_device(dev)) idno = -1; else idno = to_platform_device(dev)->id; spin_lock(&clocks_lock); list_for_each_entry(p, &clocks, list) { if (p->id == idno && strcmp(id, p->name) == 0 && try_module_get(p->owner)) { clk = p; break; } } /* check for the case where a device was supplied, but the * clock that was being searched for is not device specific */ if (IS_ERR(clk)) { list_for_each_entry(p, &clocks, list) { if (p->id == -1 && strcmp(id, p->name) == 0 && try_module_get(p->owner)) { clk = p; break; } } } same situation for mach-at91/clock.c: /* clocks cannot be de-registered no refcounting necessary */ struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; list_for_each_entry(clk, &clocks, node) { if (strcmp(id, clk->name) == 0) return clk; if (clk->function && (dev == clk->dev) && strcmp(id, clk->function) == 0) return clk; } return ERR_PTR(-ENOENT); } EXPORT_SYMBOL(clk_get); msm required device struct matched: struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; mutex_lock(&clocks_mutex); list_for_each_entry(clk, &clocks, list) if (!strcmp(id, clk->name) && clk->dev == dev) goto found_it; list_for_each_entry(clk, &clocks, list) if (!strcmp(id, clk->name) && clk->dev == NULL) goto found_it; clk = ERR_PTR(-ENOENT); found_it: mutex_unlock(&clocks_mutex); return clk; } EXPORT_SYMBOL(clk_get); > So there's no point in having the DT describe that name - that's out of > its realm. yes. i made a mistake. our old codes have an ugly implement of clk APIs, which is using names with index, for example "uart0", "uart1", "uart2", to distinguish clocks for multi-instances of same kind of devices. so i included this name in dts to bring up and test device tree functions fast. i believe our clk implementation need clean-up. but what's the best possible way to bind clocks with dynamic registerred devices from device tree? people maybe need more agreement. > > One of the problems is that clk_get() hides the mapping of device+connection > internally, which it has had to as we haven't had a device tree to look > things up. > > In essence, clk_get() is looking up a property (the clock connection name) > for the struct device. When clks get converted to the device tree, the > DT stuff should hook inside clk_get() to do a property lookup to discover > which clock the driver wants. > > Drivers should definitely not be looking up a property in the device tree > and using that as a connection name into clk_get().
WARNING: multiple messages have this Message-ID (diff)
From: 21cnbao@gmail.com (Barry Song) To: linux-arm-kernel@lists.infradead.org Subject: [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree. Date: Fri, 3 Jun 2011 10:32:52 +0800 [thread overview] Message-ID: <BANLkTikiaA3CCRAXkB=x56wtt8nNF9dqxA@mail.gmail.com> (raw) In-Reply-To: <20110602214322.GC10532@n2100.arm.linux.org.uk> 2011/6/3 Russell King - ARM Linux <linux@arm.linux.org.uk>: > On Fri, Jun 03, 2011 at 12:21:50AM +0800, Barry Song wrote: >> Arnd has required me to use device tree in our new SoC for the coming >> upstream. so i am trying to define a property like clock = "uart" in >> dts. then in drivers, >> i get this ?string by: >> clk = of_get_property(np, "clock", NULL); >> then request this clock by clk_get(). > > This is entirely wrong. ?clk_get() takes two things. ?It takes a struct > device. ?We should know what the struct device is (provided they're named > in a stable manner.) > > The other parameter, the string, is up to the driver. ?It's not a device > property. ?It's not a SoC-wide clock name. ?It's a connection name for > the clock on the device. ?This won't change from one instance of the > device to another instance of the device - it's effectively a constant. > i see :-) but there is really no an unified rule by now, for exmaple, samsung just required platform device names matched with the string parameter to get a clock. it looks like clk_get in plat-samsung depends on the string more than device and the clock name is in SoC level. struct clk *clk_get(struct device *dev, const char *id) { struct clk *p; struct clk *clk = ERR_PTR(-ENOENT); int idno; if (dev == NULL || !dev_is_platform_device(dev)) idno = -1; else idno = to_platform_device(dev)->id; spin_lock(&clocks_lock); list_for_each_entry(p, &clocks, list) { if (p->id == idno && strcmp(id, p->name) == 0 && try_module_get(p->owner)) { clk = p; break; } } /* check for the case where a device was supplied, but the * clock that was being searched for is not device specific */ if (IS_ERR(clk)) { list_for_each_entry(p, &clocks, list) { if (p->id == -1 && strcmp(id, p->name) == 0 && try_module_get(p->owner)) { clk = p; break; } } } same situation for mach-at91/clock.c: /* clocks cannot be de-registered no refcounting necessary */ struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; list_for_each_entry(clk, &clocks, node) { if (strcmp(id, clk->name) == 0) return clk; if (clk->function && (dev == clk->dev) && strcmp(id, clk->function) == 0) return clk; } return ERR_PTR(-ENOENT); } EXPORT_SYMBOL(clk_get); msm required device struct matched: struct clk *clk_get(struct device *dev, const char *id) { struct clk *clk; mutex_lock(&clocks_mutex); list_for_each_entry(clk, &clocks, list) if (!strcmp(id, clk->name) && clk->dev == dev) goto found_it; list_for_each_entry(clk, &clocks, list) if (!strcmp(id, clk->name) && clk->dev == NULL) goto found_it; clk = ERR_PTR(-ENOENT); found_it: mutex_unlock(&clocks_mutex); return clk; } EXPORT_SYMBOL(clk_get); > So there's no point in having the DT describe that name - that's out of > its realm. yes. i made a mistake. our old codes have an ugly implement of clk APIs, which is using names with index, for example "uart0", "uart1", "uart2", to distinguish clocks for multi-instances of same kind of devices. so i included this name in dts to bring up and test device tree functions fast. i believe our clk implementation need clean-up. but what's the best possible way to bind clocks with dynamic registerred devices from device tree? people maybe need more agreement. > > One of the problems is that clk_get() hides the mapping of device+connection > internally, which it has had to as we haven't had a device tree to look > things up. > > In essence, clk_get() is looking up a property (the clock connection name) > for the struct device. ?When clks get converted to the device tree, the > DT stuff should hook inside clk_get() to do a property lookup to discover > which clock the driver wants. > > Drivers should definitely not be looking up a property in the device tree > and using that as a connection name into clk_get().
next prev parent reply other threads:[~2011-06-03 2:32 UTC|newest] Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-05-27 20:56 [RFC 0/2] ARM: Tegra: Device Tree: Audio John Bonesio 2011-05-27 20:56 ` John Bonesio 2011-05-27 20:57 ` [RFC 1/2] ARM:Tegra: Device Tree Support: Initialize the audio card from the device tree John Bonesio 2011-05-27 21:05 ` Grant Likely 2011-05-27 21:05 ` Grant Likely 2011-05-28 1:28 ` Mark Brown 2011-05-28 1:28 ` Mark Brown 2011-06-01 7:07 ` Barry Song 2011-06-01 7:07 ` Barry Song [not found] ` <BANLkTi=NMjBjWVv_o3PocejhgGr8TdG+1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-01 16:47 ` Grant Likely 2011-06-01 16:47 ` Grant Likely [not found] ` <BANLkTi=PtEhxxmZo2DqvmySCmnEd3LbezQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-02 9:07 ` Barry Song 2011-06-02 9:07 ` Barry Song [not found] ` <BANLkTikWpiY_o27OF-Jxvq7iPeWzrAYOsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-02 16:04 ` Grant Likely 2011-06-02 16:04 ` Grant Likely [not found] ` <20110602160445.GA8373-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org> 2011-06-02 16:21 ` Barry Song 2011-06-02 16:21 ` Barry Song [not found] ` <BANLkTikGp_O-Wd3nMhbV+-RLeXZoWeB6eQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-02 21:43 ` Russell King - ARM Linux 2011-06-02 21:43 ` Russell King - ARM Linux [not found] ` <20110602214322.GC10532-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2011-06-03 2:32 ` Barry Song [this message] 2011-06-03 2:32 ` Barry Song [not found] ` <BANLkTikiaA3CCRAXkB=x56wtt8nNF9dqxA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-03 6:20 ` Russell King - ARM Linux 2011-06-03 6:20 ` Russell King - ARM Linux 2011-06-02 21:36 ` Russell King - ARM Linux 2011-06-02 21:36 ` Russell King - ARM Linux [not found] ` <20110602213656.GB10532-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org> 2011-06-03 1:19 ` Barry Song 2011-06-03 1:19 ` Barry Song [not found] ` <BANLkTimavC-6oMhAHHsBJ2_Em7aXi7eyNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-07 3:44 ` Barry Song 2011-06-07 3:44 ` Barry Song 2011-06-14 15:42 ` Grant Likely 2011-06-14 15:42 ` Grant Likely 2011-05-27 20:57 ` [RFC 2/2] ARM:Tegra: Device Tree Support: Initialize audio card gpio's " John Bonesio 2011-05-27 21:06 ` Grant Likely 2011-05-27 21:06 ` Grant Likely 2011-05-28 1:24 ` Mark Brown 2011-05-28 1:24 ` Mark Brown [not found] ` <20110528012427.GB5971-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2011-05-30 3:11 ` Olof Johansson 2011-05-30 3:11 ` Olof Johansson [not found] ` <BANLkTinKLcYmStvBEGDcN84BapJXe5Y5bg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-05-30 3:38 ` Mark Brown 2011-05-30 3:38 ` Mark Brown [not found] ` <20110530033826.GE4130-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2011-05-30 6:11 ` Grant Likely 2011-05-30 6:11 ` Grant Likely [not found] ` <20110530061155.GC23517-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org> 2011-05-30 6:18 ` Mitch Bradley 2011-05-30 6:18 ` Mitch Bradley [not found] ` <4DE336A1.5040509-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 2011-05-30 6:22 ` Grant Likely 2011-05-30 6:22 ` Grant Likely 2011-05-30 7:01 ` Mark Brown 2011-05-30 7:01 ` Mark Brown [not found] ` <20110530070138.GA5036-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2011-05-30 16:22 ` Grant Likely 2011-05-30 16:22 ` Grant Likely 2011-05-30 18:54 ` Segher Boessenkool 2011-05-30 18:54 ` Segher Boessenkool [not found] ` <8d2515b13c817cc956b185d043bcef00-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> 2011-05-30 19:20 ` Grant Likely 2011-05-30 19:20 ` Grant Likely [not found] ` <BANLkTi=hkScxYpt449CQCky+bLU3UvkC_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-05-30 20:53 ` Mitch Bradley 2011-05-30 20:53 ` Mitch Bradley [not found] ` <4DE403C5.7060003-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 2011-05-31 17:55 ` Stephen Warren 2011-05-31 17:55 ` Stephen Warren [not found] ` <74CDBE0F657A3D45AFBB94109FB122FF0498E1C22D-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org> 2011-05-31 18:42 ` Mitch Bradley 2011-05-31 18:42 ` Mitch Bradley [not found] ` <4DE536AD.5080200-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 2011-06-01 15:59 ` Stephen Warren 2011-06-01 15:59 ` Stephen Warren [not found] ` <74CDBE0F657A3D45AFBB94109FB122FF0498E1C3F5-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org> 2011-06-01 16:18 ` Mark Brown 2011-06-01 16:18 ` Mark Brown [not found] ` <20110601161856.GB15583-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2011-06-02 15:40 ` Grant Likely 2011-06-02 15:40 ` Grant Likely 2011-06-01 21:32 ` Mitch Bradley 2011-06-01 21:32 ` Mitch Bradley [not found] ` <4DE6AFF7.3040905-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org> 2011-06-03 21:24 ` Stephen Warren 2011-06-03 21:24 ` Stephen Warren [not found] ` <74CDBE0F657A3D45AFBB94109FB122FF0498E1C870-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org> 2011-06-04 0:25 ` Mitch Bradley 2011-06-04 0:25 ` Mitch Bradley 2011-06-02 14:59 ` Grant Likely 2011-06-02 14:59 ` Grant Likely 2011-06-02 15:40 ` Grant Likely 2011-06-02 15:40 ` Grant Likely 2011-06-28 21:39 ` Grant Likely 2011-06-28 21:39 ` Grant Likely 2011-05-30 23:27 ` Benjamin Herrenschmidt 2011-05-30 23:27 ` Benjamin Herrenschmidt 2011-05-30 23:49 ` Olof Johansson 2011-05-30 23:49 ` Olof Johansson [not found] ` <20110530234909.GA3411-O5ziIzlqnXUVNXGz7ipsyg@public.gmane.org> 2011-05-31 0:58 ` Segher Boessenkool 2011-05-31 0:58 ` Segher Boessenkool 2011-05-31 10:24 ` Mark Brown 2011-05-31 10:24 ` Mark Brown 2011-05-30 7:10 ` Mark Brown 2011-05-30 7:10 ` Mark Brown 2011-05-30 23:26 ` Benjamin Herrenschmidt 2011-05-30 23:26 ` Benjamin Herrenschmidt 2011-05-31 10:03 ` Mark Brown 2011-05-31 10:03 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='BANLkTikiaA3CCRAXkB=x56wtt8nNF9dqxA@mail.gmail.com' \ --to=21cnbao-re5jqeeqqe8avxtiumwx3w@public.gmane.org \ --cc=bones-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \ --cc=broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org \ --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \ --cc=grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=olofj-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.