From: Viresh Kumar <viresh.kumar@linaro.org> To: Rob Herring <rob.herring@linaro.org> Cc: Mike Turquette <mturquette@linaro.org>, Grant Likely <grant.likely@linaro.org>, Stephen Boyd <sboyd@codeaurora.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Shawn Guo <shawn.guo@linaro.org>, Lists linaro-kernel <linaro-kernel@lists.linaro.org>, "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Arvind Chauhan <arvind.chauhan@arm.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, linux-arm-msm@vger.kernel.org, Sachin Kamat <spk.linux@gmail.com>, Thomas P Abraham <thomas.ab@samsung.com>, Nishanth Menon <nm@ti.com>, Tomasz Figa <t.figa@samsung.com>, Mark Brown <broonie@kernel.org>, Mark Rutland <Mark.Rutland@arm.com> Subject: Re: [PATCH 2/2] cpufreq: cpu0: Extend support beyond CPU0 Date: Tue, 1 Jul 2014 16:44:04 +0530 [thread overview] Message-ID: <CAKohpokaVPn7yNPNAGrE6juGaOYuxZvVsbmkOZmyi9rDub6t7g@mail.gmail.com> (raw) In-Reply-To: <CABGGiswMuofH5WRh4jgwh1=CV+fEXM_Ec3+WNwThbuMLVxsdcg@mail.gmail.com> On 1 July 2014 00:03, Rob Herring <rob.herring@linaro.org> wrote: >> What about comparing "clocks" property in cpu DT nodes? > > What if a different clock is selected for some reason. I don't know why that will happen for CPUs sharing clock line. > I think a clock api function would be better. @Mike: What do you think? I think we can get a clock API for this. > That being said, I don't really have any issue with such a function. > Some comments on the implementation. >> +static int of_property_match(const struct device_node *np1, >> + const struct device_node *np2, >> + const char *list_name) >> +{ >> + const __be32 *list1, *list2, *list1_end; > > s/list/prop/ > > Everywhere. Ok. >> + int size1, size2; >> + phandle phandle1, phandle2; >> + >> + /* Retrieve the list property */ >> + list1 = of_get_property(np1, list_name, &size1); >> + if (!list1) >> + return -ENOENT; >> + >> + list2 = of_get_property(np2, list_name, &size2); >> + if (!list2) >> + return -ENOENT; >> + >> + if (size1 != size2) >> + return 0; >> + >> + list1_end = list1 + size1 / sizeof(*list1); >> + >> + /* Loop over the phandles */ >> + while (list1 < list1_end) { >> + phandle1 = be32_to_cpup(list1++); >> + phandle2 = be32_to_cpup(list2++); >> + >> + if (phandle1 != phandle2) >> + return 0; >> + } > > You can just do a memcmp here. Yeah, that would be much better. > This is wrong anyway because you don't know #clock-cells size. I was actually comparing all the clock-cells, whatever there number is to make sure "clocks" properties are exactly same. Anyway memcmp will still guarantee that. Thanks for your review.
WARNING: multiple messages have this Message-ID (diff)
From: viresh.kumar@linaro.org (Viresh Kumar) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/2] cpufreq: cpu0: Extend support beyond CPU0 Date: Tue, 1 Jul 2014 16:44:04 +0530 [thread overview] Message-ID: <CAKohpokaVPn7yNPNAGrE6juGaOYuxZvVsbmkOZmyi9rDub6t7g@mail.gmail.com> (raw) In-Reply-To: <CABGGiswMuofH5WRh4jgwh1=CV+fEXM_Ec3+WNwThbuMLVxsdcg@mail.gmail.com> On 1 July 2014 00:03, Rob Herring <rob.herring@linaro.org> wrote: >> What about comparing "clocks" property in cpu DT nodes? > > What if a different clock is selected for some reason. I don't know why that will happen for CPUs sharing clock line. > I think a clock api function would be better. @Mike: What do you think? I think we can get a clock API for this. > That being said, I don't really have any issue with such a function. > Some comments on the implementation. >> +static int of_property_match(const struct device_node *np1, >> + const struct device_node *np2, >> + const char *list_name) >> +{ >> + const __be32 *list1, *list2, *list1_end; > > s/list/prop/ > > Everywhere. Ok. >> + int size1, size2; >> + phandle phandle1, phandle2; >> + >> + /* Retrieve the list property */ >> + list1 = of_get_property(np1, list_name, &size1); >> + if (!list1) >> + return -ENOENT; >> + >> + list2 = of_get_property(np2, list_name, &size2); >> + if (!list2) >> + return -ENOENT; >> + >> + if (size1 != size2) >> + return 0; >> + >> + list1_end = list1 + size1 / sizeof(*list1); >> + >> + /* Loop over the phandles */ >> + while (list1 < list1_end) { >> + phandle1 = be32_to_cpup(list1++); >> + phandle2 = be32_to_cpup(list2++); >> + >> + if (phandle1 != phandle2) >> + return 0; >> + } > > You can just do a memcmp here. Yeah, that would be much better. > This is wrong anyway because you don't know #clock-cells size. I was actually comparing all the clock-cells, whatever there number is to make sure "clocks" properties are exactly same. Anyway memcmp will still guarantee that. Thanks for your review.
next prev parent reply other threads:[~2014-07-01 11:14 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-06-25 8:42 [PATCH 0/2] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar 2014-06-25 8:42 ` Viresh Kumar 2014-06-25 8:42 ` [PATCH 1/2] cpufreq: Add support for per-policy driver data Viresh Kumar 2014-06-25 8:42 ` Viresh Kumar 2014-06-25 8:42 ` [PATCH 2/2] cpufreq: cpu0: Extend support beyond CPU0 Viresh Kumar 2014-06-25 8:42 ` Viresh Kumar 2014-06-25 19:02 ` Stephen Boyd 2014-06-25 19:02 ` Stephen Boyd 2014-06-26 1:55 ` Viresh Kumar 2014-06-26 1:55 ` Viresh Kumar 2014-06-26 1:55 ` Viresh Kumar 2014-06-26 7:34 ` Viresh Kumar 2014-06-26 7:34 ` Viresh Kumar 2014-06-26 7:34 ` Viresh Kumar 2014-06-26 10:52 ` Viresh Kumar 2014-06-26 10:52 ` Viresh Kumar 2014-06-26 10:52 ` Viresh Kumar 2014-06-27 0:06 ` Stephen Boyd 2014-06-27 0:06 ` Stephen Boyd 2014-06-27 0:06 ` Stephen Boyd 2014-06-27 1:53 ` Mike Turquette 2014-06-27 1:53 ` Mike Turquette 2014-06-27 1:53 ` Mike Turquette 2014-06-27 2:15 ` Viresh Kumar 2014-06-27 2:15 ` Viresh Kumar 2014-06-27 2:15 ` Viresh Kumar 2014-06-30 7:57 ` Viresh Kumar 2014-06-30 7:57 ` Viresh Kumar 2014-06-30 7:57 ` Viresh Kumar 2014-06-30 18:33 ` Rob Herring 2014-06-30 18:33 ` Rob Herring 2014-06-30 18:33 ` Rob Herring 2014-07-01 11:14 ` Viresh Kumar [this message] 2014-07-01 11:14 ` Viresh Kumar 2014-07-01 11:14 ` Viresh Kumar 2014-07-01 22:00 ` Mike Turquette 2014-07-01 22:00 ` Mike Turquette 2014-07-01 22:00 ` Mike Turquette 2014-07-02 3:32 ` Viresh Kumar 2014-07-02 3:32 ` Viresh Kumar 2014-07-02 3:32 ` Viresh Kumar 2014-06-27 2:26 ` Viresh Kumar 2014-06-27 2:26 ` Viresh Kumar 2014-06-27 2:26 ` Viresh Kumar 2014-06-26 22:08 ` Mark Brown 2014-06-26 22:08 ` Mark Brown 2014-06-28 14:52 ` Shawn Guo 2014-06-28 14:52 ` Shawn Guo 2014-06-28 14:52 ` Shawn Guo 2014-06-30 4:50 ` Viresh Kumar 2014-06-30 4:50 ` Viresh Kumar 2014-06-30 4:50 ` Viresh Kumar
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=CAKohpokaVPn7yNPNAGrE6juGaOYuxZvVsbmkOZmyi9rDub6t7g@mail.gmail.com \ --to=viresh.kumar@linaro.org \ --cc=Mark.Rutland@arm.com \ --cc=arvind.chauhan@arm.com \ --cc=broonie@kernel.org \ --cc=grant.likely@linaro.org \ --cc=linaro-kernel@lists.linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=mturquette@linaro.org \ --cc=nm@ti.com \ --cc=rjw@rjwysocki.net \ --cc=rob.herring@linaro.org \ --cc=sboyd@codeaurora.org \ --cc=shawn.guo@linaro.org \ --cc=spk.linux@gmail.com \ --cc=t.figa@samsung.com \ --cc=thomas.ab@samsung.com \ /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.