Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Shawn Guo (2012-11-28 17:51:36) > > The notifiers in the clk framework might be a better place for this than > > just simply hacking the logic into the .set_rate callback. > > Ah, right. How did I forget about that nice piece? > > > I haven't looked at the definition of hb_voltage_change but does the > > call graph make any clk api calls? Are you talking over i2c to a > > regulator? If so then you'll probably hit the same reentrancy problem I > > hit when trying to make a general solution. > > So, how is your "reentrancy in the common clk framework" series[1] > going on? Haven't seen any update since August. > I've begun to look at a dvfs api that builds on top of the clock framework, as opposed to using clk_set_rate as the dvfs api itself. This eliminates the need for reentrancy, at least for the dvfs case. I'll post more when I have it. Honestly the reentrancy stuff was just too ugly. I might try again some day but for now I'm thinking a less radical approach deserves consideration. Thanks, Mike > Shawn > > [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
> The notifiers in the clk framework might be a better place for this than > just simply hacking the logic into the .set_rate callback. Ah, right. How did I forget about that nice piece? > I haven't looked at the definition of hb_voltage_change but does the > call graph make any clk api calls? Are you talking over i2c to a > regulator? If so then you'll probably hit the same reentrancy problem I > hit when trying to make a general solution. So, how is your "reentrancy in the common clk framework" series[1] going on? Haven't seen any update since August. Shawn [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 03:05 PM, Mike Turquette wrote: > Quoting Mark Langsdorf (2012-11-28 08:18:35) >> On 11/28/2012 10:01 AM, Mike Turquette wrote: >>> Quoting Shawn Guo (2012-11-28 07:17:44) On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: > On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: >> I'd >> have to move most of the logic of hb_set_target() into >> clk_highbank.c:clk_pll_set_rate() and then add extra logic for when >> cpufreq is not enabled/loaded. > > You only need to move hb_voltage_change() into cpu clock's .set_rate() > hook with no need of checking if cpufreq is enabled or not. > Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn >>> >>> The notifiers in the clk framework might be a better place for this than >>> just simply hacking the logic into the .set_rate callback. >> >> Unless the clk notifiers are different than the cpufreq notifiers, they >> don't handle returning error conditions very well. And given that the >> voltage change operation can fail (though it almost always succeeds on a >> retry) I need to be able to handle and detect that error condition. > > The notifier handler can handle the case where the transition fails (and > needs to be retried). > > Also you should check out the clk notifiers. I think they handle > failure decently. If a notifer returns an error code then everything > unrolls and the clk_set_rate operation aborts. Thanks for the pointer. The clk notifier calls seem to be working with cpufreq-cpu0. I did enough surgery on the code that I want to run a lot of stress tests before I resubmit. I'll try to have something for Tuesday. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Mark Langsdorf (2012-11-28 08:18:35) > On 11/28/2012 10:01 AM, Mike Turquette wrote: > > Quoting Shawn Guo (2012-11-28 07:17:44) > >> On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: > >>> On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: > I'd > have to move most of the logic of hb_set_target() into > clk_highbank.c:clk_pll_set_rate() and then add extra logic for when > cpufreq is not enabled/loaded. > >>> > >>> You only need to move hb_voltage_change() into cpu clock's .set_rate() > >>> hook with no need of checking if cpufreq is enabled or not. > >>> > >> Need to also check whether frequency or voltage should be changed first > >> in .set_rate() though. > >> > >> Shawn > >> > > > > The notifiers in the clk framework might be a better place for this than > > just simply hacking the logic into the .set_rate callback. > > Unless the clk notifiers are different than the cpufreq notifiers, they > don't handle returning error conditions very well. And given that the > voltage change operation can fail (though it almost always succeeds on a > retry) I need to be able to handle and detect that error condition. > The notifier handler can handle the case where the transition fails (and needs to be retried). Also you should check out the clk notifiers. I think they handle failure decently. If a notifer returns an error code then everything unrolls and the clk_set_rate operation aborts. Regards, Mike > > I haven't looked at the definition of hb_voltage_change but does the > > call graph make any clk api calls? Are you talking over i2c to a > > regulator? If so then you'll probably hit the same reentrancy problem I > > hit when trying to make a general solution. > > I'm talking over a pl320 Interprocessor Communication Mailbox to a > separate core running it's own RTOS. The RTOS might speak i2c to a > regulator but it's a black box to me. > > hb_voltage_change() doesn't make any clk api calls. It changes the > voltages, and then hb_set_target() makes clk api calls to change the > frequency. > > --Mark Langsdorf > Calxeda, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 10:01 AM, Mike Turquette wrote: > Quoting Shawn Guo (2012-11-28 07:17:44) >> On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: >>> On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. >>> >>> You only need to move hb_voltage_change() into cpu clock's .set_rate() >>> hook with no need of checking if cpufreq is enabled or not. >>> >> Need to also check whether frequency or voltage should be changed first >> in .set_rate() though. >> >> Shawn >> > > The notifiers in the clk framework might be a better place for this than > just simply hacking the logic into the .set_rate callback. Unless the clk notifiers are different than the cpufreq notifiers, they don't handle returning error conditions very well. And given that the voltage change operation can fail (though it almost always succeeds on a retry) I need to be able to handle and detect that error condition. > I haven't looked at the definition of hb_voltage_change but does the > call graph make any clk api calls? Are you talking over i2c to a > regulator? If so then you'll probably hit the same reentrancy problem I > hit when trying to make a general solution. I'm talking over a pl320 Interprocessor Communication Mailbox to a separate core running it's own RTOS. The RTOS might speak i2c to a regulator but it's a black box to me. hb_voltage_change() doesn't make any clk api calls. It changes the voltages, and then hb_set_target() makes clk api calls to change the frequency. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Shawn Guo (2012-11-28 07:17:44) > On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: > > On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: > > > I'd > > > have to move most of the logic of hb_set_target() into > > > clk_highbank.c:clk_pll_set_rate() and then add extra logic for when > > > cpufreq is not enabled/loaded. > > > > You only need to move hb_voltage_change() into cpu clock's .set_rate() > > hook with no need of checking if cpufreq is enabled or not. > > > Need to also check whether frequency or voltage should be changed first > in .set_rate() though. > > Shawn > The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. Regards, Mike > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 09:17 AM, Shawn Guo wrote: > On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: >> On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: >>> I'd >>> have to move most of the logic of hb_set_target() into >>> clk_highbank.c:clk_pll_set_rate() and then add extra logic for when >>> cpufreq is not enabled/loaded. >> >> You only need to move hb_voltage_change() into cpu clock's .set_rate() >> hook with no need of checking if cpufreq is enabled or not. >> > Need to also check whether frequency or voltage should be changed first > in .set_rate() though. Yes, that's entirely what I meant when I said that I would need to move most of the hb_set_target() logic into .set_rate(). I would also need to account for retries if the voltage set operation fails, which it sometimes does. The ECME handles changing the voltage but doesn't look like a voltage regulator. Amongst other things, by design it doesn't export meaningful voltage information to Linux. I tried to get cpufreq-clk0 to work with the Highbank design and it ended up being much easier and more sane to create a separate driver. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: > On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: > > I'd > > have to move most of the logic of hb_set_target() into > > clk_highbank.c:clk_pll_set_rate() and then add extra logic for when > > cpufreq is not enabled/loaded. > > You only need to move hb_voltage_change() into cpu clock's .set_rate() > hook with no need of checking if cpufreq is enabled or not. > Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: > Standard practice is to have cpufreq_set_target() handle voltage > transitions and leave clk_set_rate() handle the frequency changes. The standard practice is to have cpufreq_set_target() handle both voltage and frequency transitions, while voltage is handled by regulator and frequency by clk API. > I'd > have to move most of the logic of hb_set_target() into > clk_highbank.c:clk_pll_set_rate() and then add extra logic for when > cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. > I don't think the clk maintainers would > take that patch, either. This is all handled platform clock specific .set_rate() hook. I doubt it will concern clk maintainers at all, especially when doing so we will avoid another cpufreq driver by just using cpufreq-cpu0 driver. Shawn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/27/2012 08:32 PM, Shawn Guo wrote:
> On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
>> Highbank processors depend on the external ECME to perform voltage
>> management based on a requested frequency. Communication between the
>> A9 cores and the ECME happens over the pl320 IPC channel.
>
> ...
>
>> +static int hb_voltage_change(unsigned int freq)
>> +{
>> +int i;
>> +u32 msg[7];
>> +
>> +msg[0] = HB_CPUFREQ_CHANGE_NOTE;
>> +msg[1] = freq / 1000;
>> +for (i = 2; i < 7; i++)
>> +msg[i] = 0;
>> +
>> +return pl320_ipc_transmit(msg);
>
> Is it possible to have this handled inside clk_set_rate() call of cpu
> clock?
Standard practice is to have cpufreq_set_target() handle voltage
transitions and leave clk_set_rate() handle the frequency changes. I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded. I don't think the clk maintainers would
take that patch, either.
So no.
--Mark Langsdorf
Calxeda, Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/27/2012 08:32 PM, Shawn Guo wrote:
On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
...
+static int hb_voltage_change(unsigned int freq)
+{
+int i;
+u32 msg[7];
+
+msg[0] = HB_CPUFREQ_CHANGE_NOTE;
+msg[1] = freq / 1000;
+for (i = 2; i 7; i++)
+msg[i] = 0;
+
+return pl320_ipc_transmit(msg);
Is it possible to have this handled inside clk_set_rate() call of cpu
clock?
Standard practice is to have cpufreq_set_target() handle voltage
transitions and leave clk_set_rate() handle the frequency changes. I'd
have to move most of the logic of hb_set_target() into
clk_highbank.c:clk_pll_set_rate() and then add extra logic for when
cpufreq is not enabled/loaded. I don't think the clk maintainers would
take that patch, either.
So no.
--Mark Langsdorf
Calxeda, Inc.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: Standard practice is to have cpufreq_set_target() handle voltage transitions and leave clk_set_rate() handle the frequency changes. The standard practice is to have cpufreq_set_target() handle both voltage and frequency transitions, while voltage is handled by regulator and frequency by clk API. I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. I don't think the clk maintainers would take that patch, either. This is all handled platform clock specific .set_rate() hook. I doubt it will concern clk maintainers at all, especially when doing so we will avoid another cpufreq driver by just using cpufreq-cpu0 driver. Shawn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 09:17 AM, Shawn Guo wrote: On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Yes, that's entirely what I meant when I said that I would need to move most of the hb_set_target() logic into .set_rate(). I would also need to account for retries if the voltage set operation fails, which it sometimes does. The ECME handles changing the voltage but doesn't look like a voltage regulator. Amongst other things, by design it doesn't export meaningful voltage information to Linux. I tried to get cpufreq-clk0 to work with the Highbank design and it ended up being much easier and more sane to create a separate driver. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Shawn Guo (2012-11-28 07:17:44) On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. Regards, Mike ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 10:01 AM, Mike Turquette wrote: Quoting Shawn Guo (2012-11-28 07:17:44) On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. Unless the clk notifiers are different than the cpufreq notifiers, they don't handle returning error conditions very well. And given that the voltage change operation can fail (though it almost always succeeds on a retry) I need to be able to handle and detect that error condition. I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. I'm talking over a pl320 Interprocessor Communication Mailbox to a separate core running it's own RTOS. The RTOS might speak i2c to a regulator but it's a black box to me. hb_voltage_change() doesn't make any clk api calls. It changes the voltages, and then hb_set_target() makes clk api calls to change the frequency. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Mark Langsdorf (2012-11-28 08:18:35) On 11/28/2012 10:01 AM, Mike Turquette wrote: Quoting Shawn Guo (2012-11-28 07:17:44) On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. Unless the clk notifiers are different than the cpufreq notifiers, they don't handle returning error conditions very well. And given that the voltage change operation can fail (though it almost always succeeds on a retry) I need to be able to handle and detect that error condition. The notifier handler can handle the case where the transition fails (and needs to be retried). Also you should check out the clk notifiers. I think they handle failure decently. If a notifer returns an error code then everything unrolls and the clk_set_rate operation aborts. Regards, Mike I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. I'm talking over a pl320 Interprocessor Communication Mailbox to a separate core running it's own RTOS. The RTOS might speak i2c to a regulator but it's a black box to me. hb_voltage_change() doesn't make any clk api calls. It changes the voltages, and then hb_set_target() makes clk api calls to change the frequency. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On 11/28/2012 03:05 PM, Mike Turquette wrote: Quoting Mark Langsdorf (2012-11-28 08:18:35) On 11/28/2012 10:01 AM, Mike Turquette wrote: Quoting Shawn Guo (2012-11-28 07:17:44) On Wed, Nov 28, 2012 at 10:58:02PM +0800, Shawn Guo wrote: On Wed, Nov 28, 2012 at 07:16:12AM -0600, Mark Langsdorf wrote: I'd have to move most of the logic of hb_set_target() into clk_highbank.c:clk_pll_set_rate() and then add extra logic for when cpufreq is not enabled/loaded. You only need to move hb_voltage_change() into cpu clock's .set_rate() hook with no need of checking if cpufreq is enabled or not. Need to also check whether frequency or voltage should be changed first in .set_rate() though. Shawn The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. Unless the clk notifiers are different than the cpufreq notifiers, they don't handle returning error conditions very well. And given that the voltage change operation can fail (though it almost always succeeds on a retry) I need to be able to handle and detect that error condition. The notifier handler can handle the case where the transition fails (and needs to be retried). Also you should check out the clk notifiers. I think they handle failure decently. If a notifer returns an error code then everything unrolls and the clk_set_rate operation aborts. Thanks for the pointer. The clk notifier calls seem to be working with cpufreq-cpu0. I did enough surgery on the code that I want to run a lot of stress tests before I resubmit. I'll try to have something for Tuesday. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. Ah, right. How did I forget about that nice piece? I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. So, how is your reentrancy in the common clk framework series[1] going on? Haven't seen any update since August. Shawn [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Quoting Shawn Guo (2012-11-28 17:51:36) The notifiers in the clk framework might be a better place for this than just simply hacking the logic into the .set_rate callback. Ah, right. How did I forget about that nice piece? I haven't looked at the definition of hb_voltage_change but does the call graph make any clk api calls? Are you talking over i2c to a regulator? If so then you'll probably hit the same reentrancy problem I hit when trying to make a general solution. So, how is your reentrancy in the common clk framework series[1] going on? Haven't seen any update since August. I've begun to look at a dvfs api that builds on top of the clock framework, as opposed to using clk_set_rate as the dvfs api itself. This eliminates the need for reentrancy, at least for the dvfs case. I'll post more when I have it. Honestly the reentrancy stuff was just too ugly. I might try again some day but for now I'm thinking a less radical approach deserves consideration. Thanks, Mike Shawn [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/182198 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
> Highbank processors depend on the external ECME to perform voltage
> management based on a requested frequency. Communication between the
> A9 cores and the ECME happens over the pl320 IPC channel.
...
> +static int hb_voltage_change(unsigned int freq)
> +{
> + int i;
> + u32 msg[7];
> +
> + msg[0] = HB_CPUFREQ_CHANGE_NOTE;
> + msg[1] = freq / 1000;
> + for (i = 2; i < 7; i++)
> + msg[i] = 0;
> +
> + return pl320_ipc_transmit(msg);
Is it possible to have this handled inside clk_set_rate() call of cpu
clock?
Shawn
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
Signed-off-by: Mark Langsdorf
Cc: devicetree-disc...@lists.ozlabs.org
---
Changes from v5
Changed ipc_transmit() to pl320_ipc_transmit().
Changes from v4
Removed erroneous changes to arch/arm/Kconfig.
Removed unnecessary changes to drivers/cpufreq/Kconfig.arm
Alphabetized additions to arch/arm/mach-highbank/Kconfig
Changed ipc call and header to match new ipc location in
drivers/mailbox.
Changes from v3
None.
Changes from v2
Changed transition latency binding in code to match documentation.
Changes from v1
Added highbank specific Kconfig changes.
.../bindings/cpufreq/highbank-cpufreq.txt | 53 +
arch/arm/boot/dts/highbank.dts | 10 +
arch/arm/mach-highbank/Kconfig | 2 +
drivers/cpufreq/Kconfig.arm| 13 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/highbank-cpufreq.c | 229 +
6 files changed, 308 insertions(+)
create mode 100644
Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
create mode 100644 drivers/cpufreq/highbank-cpufreq.c
diff --git a/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
new file mode 100644
index 000..1d5a836
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
@@ -0,0 +1,53 @@
+Highbank cpufreq driver
+
+This is cpufreq driver for Calxeda ECX-1000 (highbank) processor. It is based
+on the generic cpu0 driver and uses a similar format for bindings. Since
+the EnergyCore Management Engine maintains the voltage based on the
+frequency, the voltage component of the operating points can be set to any
+arbitrary values.
+
+Both required properties listed below must be defined under node /cpus/cpu@0.
+
+Required properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
+ for details
+- transition-latency: Specify the possible maximum transition latency for
clock,
+ in unit of nanoseconds.
+
+Examples:
+
+cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ compatible = "arm,cortex-a9";
+ reg = <0>;
+ next-level-cache = <>;
+ operating-points = <
+ /* kHz ignored */
+ 79 100
+ 396000 100
+ 198000 100
+ >;
+ transition-latency = <20>;
+ };
+
+ cpu@1 {
+ compatible = "arm,cortex-a9";
+ reg = <1>;
+ next-level-cache = <>;
+ };
+
+ cpu@2 {
+ compatible = "arm,cortex-a9";
+ reg = <2>;
+ next-level-cache = <>;
+ };
+
+ cpu@3 {
+ compatible = "arm,cortex-a9";
+ reg = <3>;
+ next-level-cache = <>;
+ };
+};
diff --git a/arch/arm/boot/dts/highbank.dts b/arch/arm/boot/dts/highbank.dts
index 0c6fc34..8624c94 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -36,6 +36,16 @@
next-level-cache = <>;
clocks = <>;
clock-names = "cpu";
+ operating-points = <
+ /* kHzignored */
+130 100
+120 100
+110 100
+ 80 100
+ 40 100
+ 20 100
+ >;
+ transition-latency = <10>;
};
cpu@1 {
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 2896881..b7862da 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -1,5 +1,7 @@
config ARCH_HIGHBANK
bool "Calxeda ECX-1000 (Highbank)" if ARCH_MULTI_V7
+ select ARCH_HAS_CPUFREQ
+ select ARCH_HAS_OPP
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARM_AMBA
select ARM_GIC
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5961e64..7a8bcdc 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -76,3 +76,16 @@ config ARM_EXYNOS5250_CPUFREQ
help
This adds the CPUFreq driver for Samsung EXYNOS5250
SoC.
+
+config ARM_HIGHBANK_CPUFREQ
+ tristate "Calxeda Highbank-based"
+ depends on ARCH_HIGHBANK
+ select CPU_FREQ_TABLE
+ select PM_OPP
+
[PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
Signed-off-by: Mark Langsdorf mark.langsd...@calxeda.com
Cc: devicetree-disc...@lists.ozlabs.org
---
Changes from v5
Changed ipc_transmit() to pl320_ipc_transmit().
Changes from v4
Removed erroneous changes to arch/arm/Kconfig.
Removed unnecessary changes to drivers/cpufreq/Kconfig.arm
Alphabetized additions to arch/arm/mach-highbank/Kconfig
Changed ipc call and header to match new ipc location in
drivers/mailbox.
Changes from v3
None.
Changes from v2
Changed transition latency binding in code to match documentation.
Changes from v1
Added highbank specific Kconfig changes.
.../bindings/cpufreq/highbank-cpufreq.txt | 53 +
arch/arm/boot/dts/highbank.dts | 10 +
arch/arm/mach-highbank/Kconfig | 2 +
drivers/cpufreq/Kconfig.arm| 13 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/highbank-cpufreq.c | 229 +
6 files changed, 308 insertions(+)
create mode 100644
Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
create mode 100644 drivers/cpufreq/highbank-cpufreq.c
diff --git a/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
new file mode 100644
index 000..1d5a836
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/highbank-cpufreq.txt
@@ -0,0 +1,53 @@
+Highbank cpufreq driver
+
+This is cpufreq driver for Calxeda ECX-1000 (highbank) processor. It is based
+on the generic cpu0 driver and uses a similar format for bindings. Since
+the EnergyCore Management Engine maintains the voltage based on the
+frequency, the voltage component of the operating points can be set to any
+arbitrary values.
+
+Both required properties listed below must be defined under node /cpus/cpu@0.
+
+Required properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
+ for details
+- transition-latency: Specify the possible maximum transition latency for
clock,
+ in unit of nanoseconds.
+
+Examples:
+
+cpus {
+ #address-cells = 1;
+ #size-cells = 0;
+
+ cpu@0 {
+ compatible = arm,cortex-a9;
+ reg = 0;
+ next-level-cache = L2;
+ operating-points =
+ /* kHz ignored */
+ 79 100
+ 396000 100
+ 198000 100
+ ;
+ transition-latency = 20;
+ };
+
+ cpu@1 {
+ compatible = arm,cortex-a9;
+ reg = 1;
+ next-level-cache = L2;
+ };
+
+ cpu@2 {
+ compatible = arm,cortex-a9;
+ reg = 2;
+ next-level-cache = L2;
+ };
+
+ cpu@3 {
+ compatible = arm,cortex-a9;
+ reg = 3;
+ next-level-cache = L2;
+ };
+};
diff --git a/arch/arm/boot/dts/highbank.dts b/arch/arm/boot/dts/highbank.dts
index 0c6fc34..8624c94 100644
--- a/arch/arm/boot/dts/highbank.dts
+++ b/arch/arm/boot/dts/highbank.dts
@@ -36,6 +36,16 @@
next-level-cache = L2;
clocks = a9pll;
clock-names = cpu;
+ operating-points =
+ /* kHzignored */
+130 100
+120 100
+110 100
+ 80 100
+ 40 100
+ 20 100
+ ;
+ transition-latency = 10;
};
cpu@1 {
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 2896881..b7862da 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -1,5 +1,7 @@
config ARCH_HIGHBANK
bool Calxeda ECX-1000 (Highbank) if ARCH_MULTI_V7
+ select ARCH_HAS_CPUFREQ
+ select ARCH_HAS_OPP
select ARCH_WANT_OPTIONAL_GPIOLIB
select ARM_AMBA
select ARM_GIC
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 5961e64..7a8bcdc 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -76,3 +76,16 @@ config ARM_EXYNOS5250_CPUFREQ
help
This adds the CPUFreq driver for Samsung EXYNOS5250
SoC.
+
+config ARM_HIGHBANK_CPUFREQ
+ tristate Calxeda Highbank-based
+ depends on ARCH_HIGHBANK
+ select CPU_FREQ_TABLE
+ select PM_OPP
+ default m
+
Re: [PATCH 6/6 v6] cpufreq, highbank: add support for highbank cpufreq
On Tue, Nov 27, 2012 at 02:04:32PM -0600, Mark Langsdorf wrote:
Highbank processors depend on the external ECME to perform voltage
management based on a requested frequency. Communication between the
A9 cores and the ECME happens over the pl320 IPC channel.
...
+static int hb_voltage_change(unsigned int freq)
+{
+ int i;
+ u32 msg[7];
+
+ msg[0] = HB_CPUFREQ_CHANGE_NOTE;
+ msg[1] = freq / 1000;
+ for (i = 2; i 7; i++)
+ msg[i] = 0;
+
+ return pl320_ipc_transmit(msg);
Is it possible to have this handled inside clk_set_rate() call of cpu
clock?
Shawn
+}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
