From: Stefan Wahren <wahrenst@gmx.net> To: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>, Stefan Wahren <wahrenst@gmx.net>, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org Cc: f.fainelli@gmail.com, ptesarik@suse.com, sboyd@kernel.org, viresh.kumar@linaro.org, mturquette@baylibre.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, eric@anholt.net, bcm-kernel-feedback-list@broadcom.com, linux-clk@vger.kernel.org, mbrugger@suse.de, ssuloev@orpaltech.com Subject: Re: [PATCH 0/4] cpufreq support for the Raspberry Pi Date: Wed, 5 Jun 2019 22:15:35 +0200 [thread overview] Message-ID: <008bf9da-b2eb-8858-a58b-4252b7920856@gmx.net> (raw) In-Reply-To: <6f40f43ed32c5c519761879245423f7c371e4ae6.camel@suse.de> Hi Nicolas, >> Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: >>> Hi Stefan, >>> thanks for the review, I took note of your code comments. >>> >>> On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: >>>> Hi Nicolas, >>>> >>>> Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: >>>>> Hi all, >>>>> this series aims at adding cpufreq support to the Raspberry Pi family of >>>>> boards. >>>>> >>>>> The previous revision can be found at: >>>>> https://lkml.org/lkml/2019/5/20/431 >>>>> >>>>> The series first factors out 'pllb' from clk-bcm2385 and creates a new >>>>> clk driver that operates it over RPi's firmware interface[1]. We are >>>>> forced to do so as the firmware 'owns' the pll and we're not allowed to >>>>> change through the register interface directly as we might race with the >>>>> over-temperature and under-voltage protections provided by the firmware. >>>> it would be nice to preserve such design decision in the driver as a >>>> comment, because the cover letter usually get lost. >>>>> Next it creates a minimal cpufreq driver that populates the CPU's opp >>>>> table, and registers cpufreq-dt. Which is needed as the firmware >>>>> controls the max and min frequencies available. >>>> I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and >>>> manually enable this drivers. During boot with Raspbian rootfs i'm >>>> getting the following: >>>> >>>> [ 1.177009] cpu cpu0: failed to get clock: -2 >>>> [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 >>> This is surprising, who could be creating a platform_device for cpufreq-dt >>> apart from raspberrypi-cpufreq? Just to make things clear, you're using the >>> device tree from v5.2-rc1 (as opposed to the Raspbian one)? >> sorry my fault, i thought it already has been replaced. The behavior in >> this unexpected case is fine, since it doesn't crash. >> >> I replaced the the DTB with the mainline one, but now i'm getting this: >> >> [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: >> 600000 KHz >> [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 >> [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 > For the record this fixes it: > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index aa51756fd4d6..edb71eefe9cf 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1293,7 +1293,7 @@ static int clk_core_determine_round_nolock(struct clk_core > *core, > } else if (core->ops->round_rate) { > rate = core->ops->round_rate(core->hw, req->rate, > &req->best_parent_rate); > - if (rate < 0) > + if (IS_ERR_VALUE(rate)) > return rate; > > req->rate = rate; > > round_rate() returns a 'long' value, yet 'pllb' in rpi3b+ goes as high as > 2.8GHz, which only fits in an 'unsigned long'. This explains why I didn't see > this issue with RPI2b. Okay, i understand the problem. But the patch doesn't look like the proper fix to me. Maybe the better way is to implement determine_rate instead of round_rate in the clock driver. AFAICS this provides the necessary unsigned long. > > I'll add the patch to the series. > > Regards, > Nicolas
WARNING: multiple messages have this Message-ID (diff)
From: Stefan Wahren <wahrenst@gmx.net> To: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>, Stefan Wahren <wahrenst@gmx.net>, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org Cc: f.fainelli@gmail.com, ptesarik@suse.com, sboyd@kernel.org, viresh.kumar@linaro.org, mturquette@baylibre.com, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, eric@anholt.net, bcm-kernel-feedback-list@broadcom.com, linux-clk@vger.kernel.org, mbrugger@suse.de, ssuloev@orpaltech.com Subject: Re: [PATCH 0/4] cpufreq support for the Raspberry Pi Date: Wed, 5 Jun 2019 22:15:35 +0200 [thread overview] Message-ID: <008bf9da-b2eb-8858-a58b-4252b7920856@gmx.net> (raw) In-Reply-To: <6f40f43ed32c5c519761879245423f7c371e4ae6.camel@suse.de> Hi Nicolas, >> Am 05.06.19 um 13:00 schrieb Nicolas Saenz Julienne: >>> Hi Stefan, >>> thanks for the review, I took note of your code comments. >>> >>> On Wed, 2019-06-05 at 11:46 +0200, Stefan Wahren wrote: >>>> Hi Nicolas, >>>> >>>> Am 04.06.19 um 19:32 schrieb Nicolas Saenz Julienne: >>>>> Hi all, >>>>> this series aims at adding cpufreq support to the Raspberry Pi family of >>>>> boards. >>>>> >>>>> The previous revision can be found at: >>>>> https://lkml.org/lkml/2019/5/20/431 >>>>> >>>>> The series first factors out 'pllb' from clk-bcm2385 and creates a new >>>>> clk driver that operates it over RPi's firmware interface[1]. We are >>>>> forced to do so as the firmware 'owns' the pll and we're not allowed to >>>>> change through the register interface directly as we might race with the >>>>> over-temperature and under-voltage protections provided by the firmware. >>>> it would be nice to preserve such design decision in the driver as a >>>> comment, because the cover letter usually get lost. >>>>> Next it creates a minimal cpufreq driver that populates the CPU's opp >>>>> table, and registers cpufreq-dt. Which is needed as the firmware >>>>> controls the max and min frequencies available. >>>> I tested your series on top of Linux 5.2-rc1 with multi_v7_defconfig and >>>> manually enable this drivers. During boot with Raspbian rootfs i'm >>>> getting the following: >>>> >>>> [ 1.177009] cpu cpu0: failed to get clock: -2 >>>> [ 1.183643] cpufreq-dt: probe of cpufreq-dt failed with error -2 >>> This is surprising, who could be creating a platform_device for cpufreq-dt >>> apart from raspberrypi-cpufreq? Just to make things clear, you're using the >>> device tree from v5.2-rc1 (as opposed to the Raspbian one)? >> sorry my fault, i thought it already has been replaced. The behavior in >> this unexpected case is fine, since it doesn't crash. >> >> I replaced the the DTB with the mainline one, but now i'm getting this: >> >> [ 4.566068] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: >> 600000 KHz >> [ 4.580690] cpu cpu0: dev_pm_opp_set_rate: Invalid target frequency 0 >> [ 4.594391] cpufreq: __target_index: Failed to change cpu frequency: -22 > For the record this fixes it: > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index aa51756fd4d6..edb71eefe9cf 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1293,7 +1293,7 @@ static int clk_core_determine_round_nolock(struct clk_core > *core, > } else if (core->ops->round_rate) { > rate = core->ops->round_rate(core->hw, req->rate, > &req->best_parent_rate); > - if (rate < 0) > + if (IS_ERR_VALUE(rate)) > return rate; > > req->rate = rate; > > round_rate() returns a 'long' value, yet 'pllb' in rpi3b+ goes as high as > 2.8GHz, which only fits in an 'unsigned long'. This explains why I didn't see > this issue with RPI2b. Okay, i understand the problem. But the patch doesn't look like the proper fix to me. Maybe the better way is to implement determine_rate instead of round_rate in the clock driver. AFAICS this provides the necessary unsigned long. > > I'll add the patch to the series. > > Regards, > Nicolas _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-06-05 20:16 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-04 17:32 [PATCH 0/4] cpufreq support for the Raspberry Pi Nicolas Saenz Julienne 2019-06-04 17:32 ` Nicolas Saenz Julienne 2019-06-04 17:32 ` [PATCH 1/4] clk: bcm2835: remove pllb Nicolas Saenz Julienne 2019-06-04 17:32 ` Nicolas Saenz Julienne 2019-06-05 0:03 ` Eric Anholt 2019-06-05 0:03 ` Eric Anholt 2019-06-05 10:49 ` Stefan Wahren 2019-06-05 10:49 ` Stefan Wahren 2019-06-04 17:32 ` [PATCH 2/4] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware Nicolas Saenz Julienne 2019-06-04 17:32 ` Nicolas Saenz Julienne 2019-06-05 10:44 ` Stefan Wahren 2019-06-05 10:44 ` Stefan Wahren 2019-06-05 12:23 ` Nicolas Saenz Julienne 2019-06-05 12:23 ` Nicolas Saenz Julienne 2019-06-04 17:32 ` [PATCH 3/4] clk: bcm2835: register Raspberry Pi's firmware clk device Nicolas Saenz Julienne 2019-06-04 17:32 ` Nicolas Saenz Julienne 2019-06-05 0:00 ` Eric Anholt 2019-06-05 0:00 ` Eric Anholt 2019-06-05 9:11 ` Nicolas Saenz Julienne 2019-06-05 9:11 ` Nicolas Saenz Julienne 2019-06-05 10:01 ` Stefan Wahren 2019-06-05 10:01 ` Stefan Wahren 2019-06-04 17:32 ` [PATCH 4/4] cpufreq: add driver for Raspbery Pi Nicolas Saenz Julienne 2019-06-04 17:32 ` Nicolas Saenz Julienne 2019-06-05 0:18 ` Eric Anholt 2019-06-05 0:18 ` Eric Anholt 2019-06-05 9:12 ` Nicolas Saenz Julienne 2019-06-05 9:12 ` Nicolas Saenz Julienne 2019-06-05 11:15 ` Stefan Wahren 2019-06-05 11:15 ` Stefan Wahren 2019-06-05 9:46 ` [PATCH 0/4] cpufreq support for the Raspberry Pi Stefan Wahren 2019-06-05 9:46 ` Stefan Wahren 2019-06-05 11:00 ` Nicolas Saenz Julienne 2019-06-05 11:00 ` Nicolas Saenz Julienne 2019-06-05 11:34 ` Stefan Wahren 2019-06-05 11:34 ` Stefan Wahren 2019-06-05 12:27 ` Nicolas Saenz Julienne 2019-06-05 12:27 ` Nicolas Saenz Julienne 2019-06-05 13:12 ` Stefan Wahren 2019-06-05 13:12 ` Stefan Wahren 2019-06-05 19:11 ` Nicolas Saenz Julienne 2019-06-05 19:11 ` Nicolas Saenz Julienne 2019-06-05 20:15 ` Stefan Wahren [this message] 2019-06-05 20:15 ` Stefan Wahren
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=008bf9da-b2eb-8858-a58b-4252b7920856@gmx.net \ --to=wahrenst@gmx.net \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=eric@anholt.net \ --cc=f.fainelli@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=mbrugger@suse.de \ --cc=mturquette@baylibre.com \ --cc=nsaenzjulienne@suse.de \ --cc=ptesarik@suse.com \ --cc=rjw@rjwysocki.net \ --cc=sboyd@kernel.org \ --cc=ssuloev@orpaltech.com \ --cc=viresh.kumar@linaro.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.