From: Stephen Boyd <sboyd@kernel.org> To: "Rafael J. Wysocki" <rjw@rjwysocki.net>, Mark Rutland <mark.rutland@arm.com>, Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>, Saravana Kannan <saravanak@google.com>, Viresh Kumar <vireshk@kernel.org> Cc: Saravana Kannan <saravanak@google.com>, Georgi Djakov <georgi.djakov@linaro.org>, vincent.guittot@linaro.org, seansw@qti.qualcomm.com, daidavid1@codeaurora.org, adharmap@codeaurora.org, Rajendra Nayak <rnayak@codeaurora.org>, sibis@codeaurora.org, bjorn.andersson@linaro.org, evgreen@chromium.org, kernel-team@android.com, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables Date: Fri, 16 Aug 2019 11:21:30 -0700 [thread overview] Message-ID: <20190816182131.A692E206C1@mail.kernel.org> (raw) In-Reply-To: <20190807223111.230846-3-saravanak@google.com> Quoting Saravana Kannan (2019-08-07 15:31:10) > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index 1813f5ad5fa2..e1750033fef9 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -523,6 +523,35 @@ void dev_pm_opp_of_remove_table(struct device *dev) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); > > +static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np) > +{ > + int ret; > + u64 rate; > + u32 bw; > + > + ret = of_property_read_u64(np, "opp-hz", &rate); > + if (!ret) { > + /* > + * Rate is defined as an unsigned long in clk API, and so > + * casting explicitly to its type. Must be fixed once rate is 64 > + * bit guaranteed in clk API. > + */ > + new_opp->rate = (unsigned long)rate; > + return 0; > + } > + > + ret = of_property_read_u32(np, "opp-peak-kBps", &bw); > + if (ret) > + return ret; > + new_opp->rate = (unsigned long) bw; > + > + ret = of_property_read_u32(np, "opp-avg-kBps", &bw); > + if (!ret) I would write this as if (!of_property_read_u32(np, "opp-avg-kBps", &bw)) new_opp->avg_bw = (unsigned long) bw; because you don't care about the return value. > + new_opp->avg_bw = (unsigned long) bw; > + > + return 0; > +} > + > /** > * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) > * @opp_table: OPP table > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > index 01a500e2c40a..6bb238af9cac 100644 > --- a/drivers/opp/opp.h > +++ b/drivers/opp/opp.h > @@ -56,7 +56,8 @@ extern struct list_head opp_tables; > * @turbo: true if turbo (boost) OPP > * @suspend: true if suspend OPP > * @pstate: Device's power domain's performance state. > - * @rate: Frequency in hertz > + * @rate: Frequency in hertz OR Peak bandwidth in kilobytes per second Why is Peak capitalized? > + * @avg_bw: Average bandwidth in kilobytes per second > * @level: Performance level > * @supplies: Power supplies voltage/current values > * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's > @@ -78,6 +79,7 @@ struct dev_pm_opp { > bool suspend; > unsigned int pstate; > unsigned long rate; If you're trying to save space why not make an anonymous union here of 'rate' and 'bandwidth'? Then the code doesn't read all weird. > + unsigned long avg_bw; > unsigned int level; > > struct dev_pm_opp_supply *supplies;
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org> To: "Rafael J. Wysocki" <rjw@rjwysocki.net>, Mark Rutland <mark.rutland@arm.com>, Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>, Viresh Kumar <vireshk@kernel.org> Cc: Saravana Kannan <saravanak@google.com>, Georgi Djakov <georgi.djakov@linaro.org>, vincent.guittot@linaro.org, seansw@qti.qualcomm.com, daidavid1@codeaurora.org, adharmap@codeaurora.org, Rajendra Nayak <rnayak@codeaurora.org>, sibis@codeaurora.org, bjorn.andersson@linaro.org, evgreen@chromium.org, kernel-team@android.com, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables Date: Fri, 16 Aug 2019 11:21:30 -0700 [thread overview] Message-ID: <20190816182131.A692E206C1@mail.kernel.org> (raw) In-Reply-To: <20190807223111.230846-3-saravanak@google.com> Quoting Saravana Kannan (2019-08-07 15:31:10) > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index 1813f5ad5fa2..e1750033fef9 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -523,6 +523,35 @@ void dev_pm_opp_of_remove_table(struct device *dev) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table); > > +static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np) > +{ > + int ret; > + u64 rate; > + u32 bw; > + > + ret = of_property_read_u64(np, "opp-hz", &rate); > + if (!ret) { > + /* > + * Rate is defined as an unsigned long in clk API, and so > + * casting explicitly to its type. Must be fixed once rate is 64 > + * bit guaranteed in clk API. > + */ > + new_opp->rate = (unsigned long)rate; > + return 0; > + } > + > + ret = of_property_read_u32(np, "opp-peak-kBps", &bw); > + if (ret) > + return ret; > + new_opp->rate = (unsigned long) bw; > + > + ret = of_property_read_u32(np, "opp-avg-kBps", &bw); > + if (!ret) I would write this as if (!of_property_read_u32(np, "opp-avg-kBps", &bw)) new_opp->avg_bw = (unsigned long) bw; because you don't care about the return value. > + new_opp->avg_bw = (unsigned long) bw; > + > + return 0; > +} > + > /** > * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) > * @opp_table: OPP table > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > index 01a500e2c40a..6bb238af9cac 100644 > --- a/drivers/opp/opp.h > +++ b/drivers/opp/opp.h > @@ -56,7 +56,8 @@ extern struct list_head opp_tables; > * @turbo: true if turbo (boost) OPP > * @suspend: true if suspend OPP > * @pstate: Device's power domain's performance state. > - * @rate: Frequency in hertz > + * @rate: Frequency in hertz OR Peak bandwidth in kilobytes per second Why is Peak capitalized? > + * @avg_bw: Average bandwidth in kilobytes per second > * @level: Performance level > * @supplies: Power supplies voltage/current values > * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's > @@ -78,6 +79,7 @@ struct dev_pm_opp { > bool suspend; > unsigned int pstate; > unsigned long rate; If you're trying to save space why not make an anonymous union here of 'rate' and 'bandwidth'? Then the code doesn't read all weird. > + unsigned long avg_bw; > unsigned int level; > > struct dev_pm_opp_supply *supplies;
next prev parent reply other threads:[~2019-08-16 18:21 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-07 22:31 [PATCH v5 0/3] Introduce Bandwidth OPPs for interconnects Saravana Kannan 2019-08-07 22:31 ` [PATCH v5 1/3] dt-bindings: opp: Introduce opp-peak-kBps and opp-avg-kBps bindings Saravana Kannan 2019-08-21 20:33 ` Rob Herring 2019-08-21 20:33 ` Rob Herring 2019-08-26 20:26 ` Saravana Kannan 2019-08-26 20:26 ` Saravana Kannan 2019-08-07 22:31 ` [PATCH v5 2/3] OPP: Add support for bandwidth OPP tables Saravana Kannan 2019-08-16 18:21 ` Stephen Boyd [this message] 2019-08-16 18:21 ` Stephen Boyd 2019-08-20 22:34 ` Saravana Kannan 2019-08-20 22:34 ` Saravana Kannan 2019-08-20 6:13 ` Viresh Kumar 2019-08-20 22:27 ` Saravana Kannan 2019-08-20 22:27 ` Saravana Kannan 2019-08-20 22:36 ` Saravana Kannan 2019-08-20 22:36 ` Saravana Kannan 2019-08-21 5:24 ` Viresh Kumar 2019-08-21 5:24 ` Viresh Kumar 2019-08-21 5:26 ` Viresh Kumar 2019-08-21 5:26 ` Viresh Kumar 2019-08-21 5:23 ` Viresh Kumar 2019-08-21 5:23 ` Viresh Kumar 2019-08-07 22:31 ` [PATCH v5 3/3] OPP: Add helper function " Saravana Kannan 2019-08-16 18:22 ` Stephen Boyd 2019-08-16 18:22 ` Stephen Boyd 2019-08-15 16:19 ` [PATCH v5 0/3] Introduce Bandwidth OPPs for interconnects Georgi Djakov 2019-08-16 1:54 ` Saravana Kannan 2019-08-16 1:54 ` Saravana Kannan
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=20190816182131.A692E206C1@mail.kernel.org \ --to=sboyd@kernel.org \ --cc=adharmap@codeaurora.org \ --cc=bjorn.andersson@linaro.org \ --cc=daidavid1@codeaurora.org \ --cc=devicetree@vger.kernel.org \ --cc=evgreen@chromium.org \ --cc=georgi.djakov@linaro.org \ --cc=kernel-team@android.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=nm@ti.com \ --cc=rjw@rjwysocki.net \ --cc=rnayak@codeaurora.org \ --cc=robh+dt@kernel.org \ --cc=saravanak@google.com \ --cc=seansw@qti.qualcomm.com \ --cc=sibis@codeaurora.org \ --cc=vincent.guittot@linaro.org \ --cc=vireshk@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: 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.