From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Sekhar Nori <nsekhar@ti.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Kevin Hilman <khilman@kernel.org>,
Russell King <linux@armlinux.org.uk>,
David Lechner <david@lechnology.com>,
devicetree <devicetree@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH 2/7] soc: davinci: new genpd driver
Date: Thu, 8 Feb 2018 14:27:42 +0100 [thread overview]
Message-ID: <CAMRc=MeQUdzWfAgUCX5k5iwmyyVeCvjhw0CKauSY_1F0+SBPBQ@mail.gmail.com> (raw)
In-Reply-To: <34d3980b-2c94-1540-94f0-dc0c86743475@ti.com>
2018-02-08 13:56 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Thursday 08 February 2018 03:24 PM, Bartosz Golaszewski wrote:
>> 2018-02-08 10:30 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>>> On Wednesday 07 February 2018 07:15 PM, Bartosz Golaszewski wrote:
>>>> + /*
>>>> + * DaVinci always uses a single clock for power-management. We assume
>>>> + * it's the first one in the clocks property.
>>>> + */
>>>> + clk = of_clk_get(dev->of_node, 0);
>>>> + if (IS_ERR(clk))
>>>> + return PTR_ERR(clk);
>>>
>>> We already get this today with drivers/base/power/clock_ops.c once
>>> .con_ids list is dropped from pm_clk_notifier_block (which I think it
>>> should).
>>>
>>> If there is no reason to introduce thus functionality at this stage,
>>> perhaps we should wait till such a time when its clearly needed?
>>>
>>> Thanks,
>>> Sekhar
>>
>> If I understand correctly: once we drop the con_ids list, we end up
>> calling clk_get(dev, NULL) from pm_clk_acquire(), which matches
>> against the clock with NULL con_id, which may not necessarily be the
>> first clock in the list.
>
> Hmm, not sure of this. In __of_clk_get_by_name() called by clk_get():
>
> int index = 0;
>
> /*
> * For named clocks, first look up the name in the
> * "clock-names" property. If it cannot be found, then
> * index will be an error code, and of_clk_get() will fail.
> */
> if (name)
> index = of_property_match_string(np, "clock-names", name);
>
> So, if no con_id is provided (name == NULL), then index is set to 0
> which will always get the first clock in clocks = list.
>
But we're talking here about device tree mode. In legacy mode the
device_node pointer will be NULL, __of_clk_get_by_name() will return
-ENOENT and we'll end up calling clk_get_sys() -> clk_find(). We'll
then iterate over the clock entries and check the following:
(...)
152 if (p->con_id) {
153 if (!con_id || strcmp(p->con_id, con_id))
154 continue;
155 match += 1;
156 }
(...)
So we'll skip the first clock if it has a con_id and we passed an
empty con_id to clk_get().
Bartosz
WARNING: multiple messages have this Message-ID (diff)
From: Bartosz Golaszewski <brgl-ARrdPY/1zhM@public.gmane.org>
To: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>,
David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>,
devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Linux Kernel Mailing List
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Linux ARM
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
Bartosz Golaszewski
<bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Subject: Re: [PATCH 2/7] soc: davinci: new genpd driver
Date: Thu, 8 Feb 2018 14:27:42 +0100 [thread overview]
Message-ID: <CAMRc=MeQUdzWfAgUCX5k5iwmyyVeCvjhw0CKauSY_1F0+SBPBQ@mail.gmail.com> (raw)
In-Reply-To: <34d3980b-2c94-1540-94f0-dc0c86743475-l0cyMroinI0@public.gmane.org>
2018-02-08 13:56 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>:
> On Thursday 08 February 2018 03:24 PM, Bartosz Golaszewski wrote:
>> 2018-02-08 10:30 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>:
>>> On Wednesday 07 February 2018 07:15 PM, Bartosz Golaszewski wrote:
>>>> + /*
>>>> + * DaVinci always uses a single clock for power-management. We assume
>>>> + * it's the first one in the clocks property.
>>>> + */
>>>> + clk = of_clk_get(dev->of_node, 0);
>>>> + if (IS_ERR(clk))
>>>> + return PTR_ERR(clk);
>>>
>>> We already get this today with drivers/base/power/clock_ops.c once
>>> .con_ids list is dropped from pm_clk_notifier_block (which I think it
>>> should).
>>>
>>> If there is no reason to introduce thus functionality at this stage,
>>> perhaps we should wait till such a time when its clearly needed?
>>>
>>> Thanks,
>>> Sekhar
>>
>> If I understand correctly: once we drop the con_ids list, we end up
>> calling clk_get(dev, NULL) from pm_clk_acquire(), which matches
>> against the clock with NULL con_id, which may not necessarily be the
>> first clock in the list.
>
> Hmm, not sure of this. In __of_clk_get_by_name() called by clk_get():
>
> int index = 0;
>
> /*
> * For named clocks, first look up the name in the
> * "clock-names" property. If it cannot be found, then
> * index will be an error code, and of_clk_get() will fail.
> */
> if (name)
> index = of_property_match_string(np, "clock-names", name);
>
> So, if no con_id is provided (name == NULL), then index is set to 0
> which will always get the first clock in clocks = list.
>
But we're talking here about device tree mode. In legacy mode the
device_node pointer will be NULL, __of_clk_get_by_name() will return
-ENOENT and we'll end up calling clk_get_sys() -> clk_find(). We'll
then iterate over the clock entries and check the following:
(...)
152 if (p->con_id) {
153 if (!con_id || strcmp(p->con_id, con_id))
154 continue;
155 match += 1;
156 }
(...)
So we'll skip the first clock if it has a con_id and we passed an
empty con_id to clk_get().
Bartosz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: brgl@bgdev.pl (Bartosz Golaszewski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/7] soc: davinci: new genpd driver
Date: Thu, 8 Feb 2018 14:27:42 +0100 [thread overview]
Message-ID: <CAMRc=MeQUdzWfAgUCX5k5iwmyyVeCvjhw0CKauSY_1F0+SBPBQ@mail.gmail.com> (raw)
In-Reply-To: <34d3980b-2c94-1540-94f0-dc0c86743475@ti.com>
2018-02-08 13:56 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Thursday 08 February 2018 03:24 PM, Bartosz Golaszewski wrote:
>> 2018-02-08 10:30 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>>> On Wednesday 07 February 2018 07:15 PM, Bartosz Golaszewski wrote:
>>>> + /*
>>>> + * DaVinci always uses a single clock for power-management. We assume
>>>> + * it's the first one in the clocks property.
>>>> + */
>>>> + clk = of_clk_get(dev->of_node, 0);
>>>> + if (IS_ERR(clk))
>>>> + return PTR_ERR(clk);
>>>
>>> We already get this today with drivers/base/power/clock_ops.c once
>>> .con_ids list is dropped from pm_clk_notifier_block (which I think it
>>> should).
>>>
>>> If there is no reason to introduce thus functionality at this stage,
>>> perhaps we should wait till such a time when its clearly needed?
>>>
>>> Thanks,
>>> Sekhar
>>
>> If I understand correctly: once we drop the con_ids list, we end up
>> calling clk_get(dev, NULL) from pm_clk_acquire(), which matches
>> against the clock with NULL con_id, which may not necessarily be the
>> first clock in the list.
>
> Hmm, not sure of this. In __of_clk_get_by_name() called by clk_get():
>
> int index = 0;
>
> /*
> * For named clocks, first look up the name in the
> * "clock-names" property. If it cannot be found, then
> * index will be an error code, and of_clk_get() will fail.
> */
> if (name)
> index = of_property_match_string(np, "clock-names", name);
>
> So, if no con_id is provided (name == NULL), then index is set to 0
> which will always get the first clock in clocks = list.
>
But we're talking here about device tree mode. In legacy mode the
device_node pointer will be NULL, __of_clk_get_by_name() will return
-ENOENT and we'll end up calling clk_get_sys() -> clk_find(). We'll
then iterate over the clock entries and check the following:
(...)
152 if (p->con_id) {
153 if (!con_id || strcmp(p->con_id, con_id))
154 continue;
155 match += 1;
156 }
(...)
So we'll skip the first clock if it has a con_id and we passed an
empty con_id to clk_get().
Bartosz
next prev parent reply other threads:[~2018-02-08 13:27 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-07 13:45 [PATCH 0/7] ARM: davinci: add genpd support Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 13:45 ` [PATCH 1/7] dt-bindings: soc: new driver for DaVinci genpd Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 21:47 ` David Lechner
2018-02-07 21:47 ` David Lechner
2018-02-08 8:56 ` Bartosz Golaszewski
2018-02-08 8:56 ` Bartosz Golaszewski
2018-02-08 8:56 ` Bartosz Golaszewski
2018-02-09 0:40 ` Kevin Hilman
2018-02-09 0:40 ` Kevin Hilman
2018-02-09 0:40 ` Kevin Hilman
2018-02-07 13:45 ` [PATCH 2/7] soc: davinci: new genpd driver Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 21:49 ` David Lechner
2018-02-07 21:49 ` David Lechner
2018-02-07 21:49 ` David Lechner
2018-02-08 9:30 ` Sekhar Nori
2018-02-08 9:30 ` Sekhar Nori
2018-02-08 9:30 ` Sekhar Nori
2018-02-08 9:54 ` Bartosz Golaszewski
2018-02-08 9:54 ` Bartosz Golaszewski
2018-02-08 12:56 ` Sekhar Nori
2018-02-08 12:56 ` Sekhar Nori
2018-02-08 12:56 ` Sekhar Nori
2018-02-08 13:27 ` Bartosz Golaszewski [this message]
2018-02-08 13:27 ` Bartosz Golaszewski
2018-02-08 13:27 ` Bartosz Golaszewski
2018-02-08 14:43 ` Sekhar Nori
2018-02-08 14:43 ` Sekhar Nori
2018-02-08 14:43 ` Sekhar Nori
2018-02-07 13:45 ` [PATCH 3/7] ARM: davinci: don't setup pm_clk if we're using genpd Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 13:45 ` [PATCH 4/7] ARM: dts: da850: add power controller nodes Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 21:58 ` David Lechner
2018-02-07 21:58 ` David Lechner
2018-02-07 21:58 ` David Lechner
2018-02-07 13:45 ` [PATCH 5/7] ARM: dts: da850: add power-domains properties to device nodes Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 22:21 ` David Lechner
2018-02-07 22:21 ` David Lechner
2018-02-07 13:45 ` [PATCH 6/7] ARM: davinci: select generic power domains for DaVinci in DT mode Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 13:45 ` [PATCH 7/7] ARM: davinci_all_defconfig: select the DaVinci genpd driver " Bartosz Golaszewski
2018-02-07 13:45 ` Bartosz Golaszewski
2018-02-07 22:43 ` [PATCH 0/7] ARM: davinci: add genpd support David Lechner
2018-02-07 22:43 ` David Lechner
2018-02-09 12:42 ` Sekhar Nori
2018-02-09 12:42 ` Sekhar Nori
2018-02-09 12:42 ` Sekhar Nori
2018-02-18 3:41 ` David Lechner
2018-02-18 3:41 ` David Lechner
2018-02-18 3:41 ` David Lechner
2018-02-19 10:49 ` Bartosz Golaszewski
2018-02-19 10:49 ` Bartosz Golaszewski
2018-02-19 10:49 ` Bartosz Golaszewski
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='CAMRc=MeQUdzWfAgUCX5k5iwmyyVeCvjhw0CKauSY_1F0+SBPBQ@mail.gmail.com' \
--to=brgl@bgdev.pl \
--cc=bgolaszewski@baylibre.com \
--cc=david@lechnology.com \
--cc=devicetree@vger.kernel.org \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=mark.rutland@arm.com \
--cc=nsekhar@ti.com \
--cc=robh+dt@kernel.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: link
Be 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.