| Message ID | 20190408075924.2284-2-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: da850: enable cpufreq in DT mode | expand |
On 4/8/19 2:59 AM, Bartosz Golaszewski wrote: > From: David Lechner <david@lechnology.com> > > This adds a cpu node and operating points to the common da850.dtsi file. > > Additionally, a regulator is added to the LEGO EV3 board along with > some board-specific CPU configuration. > > Regulators need to be hooked up on other boards to get them working. > I still have the same comments that I made on v1. It would be nice to also mention the LCDK board in the commit message since it is included in this patch. Also, the 415MHz operating point is something I just made up for testing and I'm not sure that it is actually useful in general. It could be omitted from this patch.
pon., 8 kwi 2019 o 15:47 David Lechner <david@lechnology.com> napisał(a): > > On 4/8/19 2:59 AM, Bartosz Golaszewski wrote: > > From: David Lechner <david@lechnology.com> > > > > This adds a cpu node and operating points to the common da850.dtsi file. > > > > Additionally, a regulator is added to the LEGO EV3 board along with > > some board-specific CPU configuration. > > > > Regulators need to be hooked up on other boards to get them working. > > > > I still have the same comments that I made on v1. It would be nice to > also mention the LCDK board in the commit message since it is included > in this patch. > My bad, I forgot it. > Also, the 415MHz operating point is something I just made up for testing > and I'm not sure that it is actually useful in general. It could be > omitted from this patch. > The board file has the 408 opp, I guess this is in line with other operating points defined by you which have slightly higher frequencies than those in the da850.c. Let's see what Sekhar thinks. Bart
On 4/8/19 8:51 AM, Bartosz Golaszewski wrote: > pon., 8 kwi 2019 o 15:47 David Lechner <david@lechnology.com> napisał(a): >> >> On 4/8/19 2:59 AM, Bartosz Golaszewski wrote: >>> From: David Lechner <david@lechnology.com> >>> >>> This adds a cpu node and operating points to the common da850.dtsi file. >>> >>> Additionally, a regulator is added to the LEGO EV3 board along with >>> some board-specific CPU configuration. >>> >>> Regulators need to be hooked up on other boards to get them working. >>> >> >> I still have the same comments that I made on v1. It would be nice to >> also mention the LCDK board in the commit message since it is included >> in this patch. >> > > My bad, I forgot it. > >> Also, the 415MHz operating point is something I just made up for testing >> and I'm not sure that it is actually useful in general. It could be >> omitted from this patch. >> > > The board file has the 408 opp, I guess this is in line with other > operating points defined by you which have slightly higher frequencies > than those in the da850.c. Let's see what Sekhar thinks. > > Bart > My bad this time. I guess I put more thought into it that I remembered. :-)
Hi Bartosz, On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > From: David Lechner <david@lechnology.com> > > This adds a cpu node and operating points to the common da850.dtsi file. > > Additionally, a regulator is added to the LEGO EV3 board along with > some board-specific CPU configuration. > > Regulators need to be hooked up on other boards to get them working. > > Signed-off-by: David Lechner <david@lechnology.com> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> I remember you mentioning about some problems using OCHI and cpufreq together. Are those resolved now? CPU PLL on DA850 can affect other peripheral clock frequencies too. So enabling it should really be a per-board decision. No problems with adding OPPs to da850.dtsi, but which of those are enabled on any board should be after some thorough testing and analysis. Because of that, I think its also better to split da850.dtsi from board specific changes in this patch. > + opp_table: opp-table { > + compatible = "operating-points-v2"; > + > + opp_100: opp100-100000000 { > + opp-hz = /bits/ 64 <100000000>; > + opp-microvolt = <1000000 950000 1050000>; > + }; > + > + opp_200: opp110-200000000 { > + opp-hz = /bits/ 64 <200000000>; > + opp-microvolt = <1100000 1050000 1160000>; > + }; > + > + opp_300: opp120-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + opp-microvolt = <1200000 1140000 1320000>; > + }; > + > + /* > + * Original silicon was 300MHz max, so higher frequencies > + * need to be enabled on a per-board basis if the chip is > + * capable. > + */ > + > + opp_375: opp120-375000000 { > + status = "disabled"; > + opp-hz = /bits/ 64 <375000000>; > + opp-microvolt = <1200000 1140000 1320000>; > + }; > + > + opp_415: opp130-415000000 { > + status = "disabled"; > + opp-hz = /bits/ 64 <415000000>; > + opp-microvolt = <1300000 1250000 1350000>; > + }; > + > + opp_456: opp130-456000000 { > + status = "disabled"; > + opp-hz = /bits/ 64 <456000000>; > + opp-microvolt = <1300000 1250000 1350000>; > + }; Here, lets stick to OPPs defined in OMAP-L138 data manual (irrespective of what existing board code has). Page 93 of http://www.ti.com/lit/ds/symlink/omap-l138.pdf Thanks, Sekhar
pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > > Hi Bartosz, > > On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > > From: David Lechner <david@lechnology.com> > > > > This adds a cpu node and operating points to the common da850.dtsi file. > > > > Additionally, a regulator is added to the LEGO EV3 board along with > > some board-specific CPU configuration. > > > > Regulators need to be hooked up on other boards to get them working. > > > > Signed-off-by: David Lechner <david@lechnology.com> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > I remember you mentioning about some problems using OCHI and cpufreq > together. Are those resolved now? CPU PLL on DA850 can affect other > peripheral clock frequencies too. So enabling it should really be a > per-board decision. > The problems are still there. I've never been able to find the culprit, but it also occurs on TI BSP in the same way (a couple cpufreq transitions will make the controller unresponsive). > No problems with adding OPPs to da850.dtsi, but which of those are > enabled on any board should be after some thorough testing and analysis. > > Because of that, I think its also better to split da850.dtsi from board > specific changes in this patch. > Sure, I'll split it. > > + opp_table: opp-table { > > + compatible = "operating-points-v2"; > > + > > + opp_100: opp100-100000000 { > > + opp-hz = /bits/ 64 <100000000>; > > + opp-microvolt = <1000000 950000 1050000>; > > + }; > > + > > + opp_200: opp110-200000000 { > > + opp-hz = /bits/ 64 <200000000>; > > + opp-microvolt = <1100000 1050000 1160000>; > > + }; > > + > > + opp_300: opp120-300000000 { > > + opp-hz = /bits/ 64 <300000000>; > > + opp-microvolt = <1200000 1140000 1320000>; > > + }; > > + > > + /* > > + * Original silicon was 300MHz max, so higher frequencies > > + * need to be enabled on a per-board basis if the chip is > > + * capable. > > + */ > > + > > + opp_375: opp120-375000000 { > > + status = "disabled"; > > + opp-hz = /bits/ 64 <375000000>; > > + opp-microvolt = <1200000 1140000 1320000>; > > + }; > > + > > + opp_415: opp130-415000000 { > > + status = "disabled"; > > + opp-hz = /bits/ 64 <415000000>; > > + opp-microvolt = <1300000 1250000 1350000>; > > + }; > > + > > + opp_456: opp130-456000000 { > > + status = "disabled"; > > + opp-hz = /bits/ 64 <456000000>; > > + opp-microvolt = <1300000 1250000 1350000>; > > + }; > > Here, lets stick to OPPs defined in OMAP-L138 data manual (irrespective > of what existing board code has). Page 93 of > http://www.ti.com/lit/ds/symlink/omap-l138.pdf > Will do in the next version. Bart > Thanks, > Sekhar
On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): >> >> Hi Bartosz, >> >> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: >>> From: David Lechner <david@lechnology.com> >>> >>> This adds a cpu node and operating points to the common da850.dtsi file. >>> >>> Additionally, a regulator is added to the LEGO EV3 board along with >>> some board-specific CPU configuration. >>> >>> Regulators need to be hooked up on other boards to get them working. >>> >>> Signed-off-by: David Lechner <david@lechnology.com> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> >> I remember you mentioning about some problems using OCHI and cpufreq >> together. Are those resolved now? CPU PLL on DA850 can affect other >> peripheral clock frequencies too. So enabling it should really be a >> per-board decision. >> > > The problems are still there. I've never been able to find the > culprit, but it also occurs on TI BSP in the same way (a couple > cpufreq transitions will make the controller unresponsive). Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK in TI BSP. If the OHCI problem is present on LCDK, then there is a user visible regression on mainline after this patch. Lets enable cpufreq in LCDK only if all working peripherals keep working afterwards. Thanks, Sekhar
pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): > > On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > > pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > >> > >> Hi Bartosz, > >> > >> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > >>> From: David Lechner <david@lechnology.com> > >>> > >>> This adds a cpu node and operating points to the common da850.dtsi file. > >>> > >>> Additionally, a regulator is added to the LEGO EV3 board along with > >>> some board-specific CPU configuration. > >>> > >>> Regulators need to be hooked up on other boards to get them working. > >>> > >>> Signed-off-by: David Lechner <david@lechnology.com> > >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >> > >> I remember you mentioning about some problems using OCHI and cpufreq > >> together. Are those resolved now? CPU PLL on DA850 can affect other > >> peripheral clock frequencies too. So enabling it should really be a > >> per-board decision. > >> > > > > The problems are still there. I've never been able to find the > > culprit, but it also occurs on TI BSP in the same way (a couple > > cpufreq transitions will make the controller unresponsive). > > Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK > in TI BSP. > Yes, I just verified that the bug occurs on LCDK with patches from this series. > If the OHCI problem is present on LCDK, then there is a user visible > regression on mainline after this patch. Lets enable cpufreq in LCDK > only if all working peripherals keep working afterwards. > The OHCI driver doesn't register any cpufreq transition notifier callbacks. I can't really find anything in the datasheet, but I'm wondering if we shouldn't do something similar to what the driver for davinci i2c controller does. I'll try a couple things tomorrow. Bart
On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): >> >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): >>>> >>>> Hi Bartosz, >>>> >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: >>>>> From: David Lechner <david@lechnology.com> >>>>> >>>>> This adds a cpu node and operating points to the common da850.dtsi file. >>>>> >>>>> Additionally, a regulator is added to the LEGO EV3 board along with >>>>> some board-specific CPU configuration. >>>>> >>>>> Regulators need to be hooked up on other boards to get them working. >>>>> >>>>> Signed-off-by: David Lechner <david@lechnology.com> >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>> >>>> I remember you mentioning about some problems using OCHI and cpufreq >>>> together. Are those resolved now? CPU PLL on DA850 can affect other >>>> peripheral clock frequencies too. So enabling it should really be a >>>> per-board decision. >>>> >>> >>> The problems are still there. I've never been able to find the >>> culprit, but it also occurs on TI BSP in the same way (a couple >>> cpufreq transitions will make the controller unresponsive). >> >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK >> in TI BSP. >> > > Yes, I just verified that the bug occurs on LCDK with patches from this series. > >> If the OHCI problem is present on LCDK, then there is a user visible >> regression on mainline after this patch. Lets enable cpufreq in LCDK >> only if all working peripherals keep working afterwards. >> > > The OHCI driver doesn't register any cpufreq transition notifier > callbacks. I can't really find anything in the datasheet, but I'm > wondering if we shouldn't do something similar to what the driver for > davinci i2c controller does. I'll try a couple things tomorrow. Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am not sure the benefits of just frequency scaling will be justifiable enough. Fixing the OHCI issue may help in other boards like da850-evm use it though. So that will be a good thing. How do you feel about keeping all OPPs disabled by default in da850.dtsi and enabling only the ones that make sense for a board in <board>.dts? Empty OPP table is illegal, so this does mean that every board must enable at least one OPP. Thanks, Sekhar
pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): > > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): > >> > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > >>>> > >>>> Hi Bartosz, > >>>> > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > >>>>> From: David Lechner <david@lechnology.com> > >>>>> > >>>>> This adds a cpu node and operating points to the common da850.dtsi file. > >>>>> > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with > >>>>> some board-specific CPU configuration. > >>>>> > >>>>> Regulators need to be hooked up on other boards to get them working. > >>>>> > >>>>> Signed-off-by: David Lechner <david@lechnology.com> > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >>>> > >>>> I remember you mentioning about some problems using OCHI and cpufreq > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other > >>>> peripheral clock frequencies too. So enabling it should really be a > >>>> per-board decision. > >>>> > >>> > >>> The problems are still there. I've never been able to find the > >>> culprit, but it also occurs on TI BSP in the same way (a couple > >>> cpufreq transitions will make the controller unresponsive). > >> > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK > >> in TI BSP. > >> > > > > Yes, I just verified that the bug occurs on LCDK with patches from this series. > > > >> If the OHCI problem is present on LCDK, then there is a user visible > >> regression on mainline after this patch. Lets enable cpufreq in LCDK > >> only if all working peripherals keep working afterwards. > >> > > > > The OHCI driver doesn't register any cpufreq transition notifier > > callbacks. I can't really find anything in the datasheet, but I'm > > wondering if we shouldn't do something similar to what the driver for > > davinci i2c controller does. I'll try a couple things tomorrow. > > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am > not sure the benefits of just frequency scaling will be justifiable enough. > > Fixing the OHCI issue may help in other boards like da850-evm use it > though. So that will be a good thing. > > How do you feel about keeping all OPPs disabled by default in da850.dtsi > and enabling only the ones that make sense for a board in <board>.dts? > > Empty OPP table is illegal, so this does mean that every board must > enable at least one OPP. > I guess in that case we'd need to make 300 MHz the default for da850-evm, but on da850-lcdk I'd go for 456 MHz (due to the regulator being fixed anyway). Bart
On Mon, Apr 15, 2019 at 6:55 AM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): > > > > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: > > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): > > >> > > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > > >>>> > > >>>> Hi Bartosz, > > >>>> > > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > > >>>>> From: David Lechner <david@lechnology.com> > > >>>>> > > >>>>> This adds a cpu node and operating points to the common da850.dtsi file. > > >>>>> > > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with > > >>>>> some board-specific CPU configuration. > > >>>>> > > >>>>> Regulators need to be hooked up on other boards to get them working. > > >>>>> > > >>>>> Signed-off-by: David Lechner <david@lechnology.com> > > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > >>>> > > >>>> I remember you mentioning about some problems using OCHI and cpufreq > > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other > > >>>> peripheral clock frequencies too. So enabling it should really be a > > >>>> per-board decision. > > >>>> > > >>> > > >>> The problems are still there. I've never been able to find the > > >>> culprit, but it also occurs on TI BSP in the same way (a couple > > >>> cpufreq transitions will make the controller unresponsive). > > >> > > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK > > >> in TI BSP. > > >> > > > > > > Yes, I just verified that the bug occurs on LCDK with patches from this series. > > > > > >> If the OHCI problem is present on LCDK, then there is a user visible > > >> regression on mainline after this patch. Lets enable cpufreq in LCDK > > >> only if all working peripherals keep working afterwards. > > >> > > > > > > The OHCI driver doesn't register any cpufreq transition notifier > > > callbacks. I can't really find anything in the datasheet, but I'm > > > wondering if we shouldn't do something similar to what the driver for > > > davinci i2c controller does. I'll try a couple things tomorrow. > > > > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am > > not sure the benefits of just frequency scaling will be justifiable enough. > > > > Fixing the OHCI issue may help in other boards like da850-evm use it > > though. So that will be a good thing. > > > > How do you feel about keeping all OPPs disabled by default in da850.dtsi > > and enabling only the ones that make sense for a board in <board>.dts? > > > > Empty OPP table is illegal, so this does mean that every board must > > enable at least one OPP. > > > > I guess in that case we'd need to make 300 MHz the default for > da850-evm, but on da850-lcdk I'd go for 456 MHz (due to the regulator > being fixed anyway). Based on what I've seen of your V4 patch, I think it's fine to let the da850-evm run at 375. I am not sure why it would need to be default to 300. I agree that the cpu-supply is correctly referencing vdcdc3_reg, and it's currently setup to handle .95 to 1.35V, which I think is OK based on what I've seen and the OPP's look right from what I can tell. adam > > Bart
pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): > > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): > >> > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > >>>> > >>>> Hi Bartosz, > >>>> > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > >>>>> From: David Lechner <david@lechnology.com> > >>>>> > >>>>> This adds a cpu node and operating points to the common da850.dtsi file. > >>>>> > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with > >>>>> some board-specific CPU configuration. > >>>>> > >>>>> Regulators need to be hooked up on other boards to get them working. > >>>>> > >>>>> Signed-off-by: David Lechner <david@lechnology.com> > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > >>>> > >>>> I remember you mentioning about some problems using OCHI and cpufreq > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other > >>>> peripheral clock frequencies too. So enabling it should really be a > >>>> per-board decision. > >>>> > >>> > >>> The problems are still there. I've never been able to find the > >>> culprit, but it also occurs on TI BSP in the same way (a couple > >>> cpufreq transitions will make the controller unresponsive). > >> > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK > >> in TI BSP. > >> > > > > Yes, I just verified that the bug occurs on LCDK with patches from this series. > > > >> If the OHCI problem is present on LCDK, then there is a user visible > >> regression on mainline after this patch. Lets enable cpufreq in LCDK > >> only if all working peripherals keep working afterwards. > >> > > > > The OHCI driver doesn't register any cpufreq transition notifier > > callbacks. I can't really find anything in the datasheet, but I'm > > wondering if we shouldn't do something similar to what the driver for > > davinci i2c controller does. I'll try a couple things tomorrow. > > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am > not sure the benefits of just frequency scaling will be justifiable enough. > > Fixing the OHCI issue may help in other boards like da850-evm use it > though. So that will be a good thing. > I've been trying different things, like suspending the device before the transition, resetting the controller or playing with the clock during transitions but it always results in the same kind of error: ohci-da8xx 1e25000.usb: frame counter not updating; disabled ohci-da8xx 1e25000.usb: HC died; cleaning up usb 1-1: USB disconnect, device number 2 If you have any idea - let me know, otherwise I'll give up. If we agree on the direction of these patches, then I can go with a single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz enabled for da850-evm. David - do you want to keep the lego board as is? Bart > How do you feel about keeping all OPPs disabled by default in da850.dtsi > and enabling only the ones that make sense for a board in <board>.dts? > > Empty OPP table is illegal, so this does mean that every board must > enable at least one OPP. > > Thanks, > Sekhar
On Tue, Apr 16, 2019 at 3:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): > > > > On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: > > > pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): > > >> > > >> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: > > >>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): > > >>>> > > >>>> Hi Bartosz, > > >>>> > > >>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: > > >>>>> From: David Lechner <david@lechnology.com> > > >>>>> > > >>>>> This adds a cpu node and operating points to the common da850.dtsi file. > > >>>>> > > >>>>> Additionally, a regulator is added to the LEGO EV3 board along with > > >>>>> some board-specific CPU configuration. > > >>>>> > > >>>>> Regulators need to be hooked up on other boards to get them working. > > >>>>> > > >>>>> Signed-off-by: David Lechner <david@lechnology.com> > > >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > >>>> > > >>>> I remember you mentioning about some problems using OCHI and cpufreq > > >>>> together. Are those resolved now? CPU PLL on DA850 can affect other > > >>>> peripheral clock frequencies too. So enabling it should really be a > > >>>> per-board decision. > > >>>> > > >>> > > >>> The problems are still there. I've never been able to find the > > >>> culprit, but it also occurs on TI BSP in the same way (a couple > > >>> cpufreq transitions will make the controller unresponsive). > > >> > > >> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK > > >> in TI BSP. > > >> > > > > > > Yes, I just verified that the bug occurs on LCDK with patches from this series. > > > > > >> If the OHCI problem is present on LCDK, then there is a user visible > > >> regression on mainline after this patch. Lets enable cpufreq in LCDK > > >> only if all working peripherals keep working afterwards. > > >> > > > > > > The OHCI driver doesn't register any cpufreq transition notifier > > > callbacks. I can't really find anything in the datasheet, but I'm > > > wondering if we shouldn't do something similar to what the driver for > > > davinci i2c controller does. I'll try a couple things tomorrow. > > > > Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am > > not sure the benefits of just frequency scaling will be justifiable enough. > > > > Fixing the OHCI issue may help in other boards like da850-evm use it > > though. So that will be a good thing. > > > > I've been trying different things, like suspending the device before > the transition, resetting the controller or playing with the clock > during transitions but it always results in the same kind of error: > > ohci-da8xx 1e25000.usb: frame counter not updating; disabled > ohci-da8xx 1e25000.usb: HC died; cleaning up > usb 1-1: USB disconnect, device number 2 > > If you have any idea - let me know, otherwise I'll give up. > > If we agree on the direction of these patches, then I can go with a > single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz > enabled for da850-evm. One last questions, and this probably directed at Sekhar, but what happens if you modify the OPP for the boards with fixed regulators to enable all the frequencies but with the only available voltage. Is there harm is running the processor at a higher voltage than necessary? I did some quick experiments on a different ARM board and I saw some changes in power consumption. I would think some power savings might be better than none, but I don't know if it causes damage. adam > > David - do you want to keep the lego board as is? > > Bart > > > How do you feel about keeping all OPPs disabled by default in da850.dtsi > > and enabling only the ones that make sense for a board in <board>.dts? > > > > Empty OPP table is illegal, so this does mean that every board must > > enable at least one OPP. > > > > Thanks, > > Sekhar
On 4/16/19 3:37 AM, Bartosz Golaszewski wrote: > pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): >> >> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: >>> pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): >>>> >>>> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: >>>>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): >>>>>> >>>>>> Hi Bartosz, >>>>>> >>>>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: >>>>>>> From: David Lechner <david@lechnology.com> >>>>>>> >>>>>>> This adds a cpu node and operating points to the common da850.dtsi file. >>>>>>> >>>>>>> Additionally, a regulator is added to the LEGO EV3 board along with >>>>>>> some board-specific CPU configuration. >>>>>>> >>>>>>> Regulators need to be hooked up on other boards to get them working. >>>>>>> >>>>>>> Signed-off-by: David Lechner <david@lechnology.com> >>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>>>> >>>>>> I remember you mentioning about some problems using OCHI and cpufreq >>>>>> together. Are those resolved now? CPU PLL on DA850 can affect other >>>>>> peripheral clock frequencies too. So enabling it should really be a >>>>>> per-board decision. >>>>>> >>>>> >>>>> The problems are still there. I've never been able to find the >>>>> culprit, but it also occurs on TI BSP in the same way (a couple >>>>> cpufreq transitions will make the controller unresponsive). >>>> >>>> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK >>>> in TI BSP. >>>> >>> >>> Yes, I just verified that the bug occurs on LCDK with patches from this series. >>> >>>> If the OHCI problem is present on LCDK, then there is a user visible >>>> regression on mainline after this patch. Lets enable cpufreq in LCDK >>>> only if all working peripherals keep working afterwards. >>>> >>> >>> The OHCI driver doesn't register any cpufreq transition notifier >>> callbacks. I can't really find anything in the datasheet, but I'm >>> wondering if we shouldn't do something similar to what the driver for >>> davinci i2c controller does. I'll try a couple things tomorrow. >> >> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am >> not sure the benefits of just frequency scaling will be justifiable enough. >> >> Fixing the OHCI issue may help in other boards like da850-evm use it >> though. So that will be a good thing. >> > > I've been trying different things, like suspending the device before > the transition, resetting the controller or playing with the clock > during transitions but it always results in the same kind of error: > > ohci-da8xx 1e25000.usb: frame counter not updating; disabled > ohci-da8xx 1e25000.usb: HC died; cleaning up > usb 1-1: USB disconnect, device number 2 > > If you have any idea - let me know, otherwise I'll give up. > > If we agree on the direction of these patches, then I can go with a > single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz > enabled for da850-evm. > > David - do you want to keep the lego board as is? Yes, I think so. Even if we can't use CPU frequency throttling because of the USB issue, I think it would still be nice if we could use this to increase the frequency once during early boot so that we can run faster than what the bootloader set. I've actually been running one of my EV3 bricks at 456MHz for almost a year now without noticing any problems. (This is of course out of spec since we only have 1.2V so I'm not saying we should add that operating point to the mainline kernel.) > > Bart > >> How do you feel about keeping all OPPs disabled by default in da850.dtsi >> and enabling only the ones that make sense for a board in <board>.dts? >> >> Empty OPP table is illegal, so this does mean that every board must >> enable at least one OPP. >> >> Thanks, >> Sekhar
On 16/04/19 5:18 PM, Adam Ford wrote: > On Tue, Apr 16, 2019 at 3:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >> pon., 15 kwi 2019 o 12:21 Sekhar Nori <nsekhar@ti.com> napisał(a): >>> >>> On 12/04/19 9:01 PM, Bartosz Golaszewski wrote: >>>> pt., 12 kwi 2019 o 15:53 Sekhar Nori <nsekhar@ti.com> napisał(a): >>>>> >>>>> On 12/04/19 5:41 PM, Bartosz Golaszewski wrote: >>>>>> pt., 12 kwi 2019 o 13:26 Sekhar Nori <nsekhar@ti.com> napisał(a): >>>>>>> >>>>>>> Hi Bartosz, >>>>>>> >>>>>>> On 08/04/19 1:29 PM, Bartosz Golaszewski wrote: >>>>>>>> From: David Lechner <david@lechnology.com> >>>>>>>> >>>>>>>> This adds a cpu node and operating points to the common da850.dtsi file. >>>>>>>> >>>>>>>> Additionally, a regulator is added to the LEGO EV3 board along with >>>>>>>> some board-specific CPU configuration. >>>>>>>> >>>>>>>> Regulators need to be hooked up on other boards to get them working. >>>>>>>> >>>>>>>> Signed-off-by: David Lechner <david@lechnology.com> >>>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>>>>> >>>>>>> I remember you mentioning about some problems using OCHI and cpufreq >>>>>>> together. Are those resolved now? CPU PLL on DA850 can affect other >>>>>>> peripheral clock frequencies too. So enabling it should really be a >>>>>>> per-board decision. >>>>>>> >>>>>> >>>>>> The problems are still there. I've never been able to find the >>>>>> culprit, but it also occurs on TI BSP in the same way (a couple >>>>>> cpufreq transitions will make the controller unresponsive). >>>>> >>>>> Is that on LCDK as well? As I recall cpufreq was never enabled on LCDK >>>>> in TI BSP. >>>>> >>>> >>>> Yes, I just verified that the bug occurs on LCDK with patches from this series. >>>> >>>>> If the OHCI problem is present on LCDK, then there is a user visible >>>>> regression on mainline after this patch. Lets enable cpufreq in LCDK >>>>> only if all working peripherals keep working afterwards. >>>>> >>>> >>>> The OHCI driver doesn't register any cpufreq transition notifier >>>> callbacks. I can't really find anything in the datasheet, but I'm >>>> wondering if we shouldn't do something similar to what the driver for >>>> davinci i2c controller does. I'll try a couple things tomorrow. >>> >>> Even if OHCI issue is fixed, with a fixed regulator like on LCDK, I am >>> not sure the benefits of just frequency scaling will be justifiable enough. >>> >>> Fixing the OHCI issue may help in other boards like da850-evm use it >>> though. So that will be a good thing. >>> >> >> I've been trying different things, like suspending the device before >> the transition, resetting the controller or playing with the clock >> during transitions but it always results in the same kind of error: >> >> ohci-da8xx 1e25000.usb: frame counter not updating; disabled >> ohci-da8xx 1e25000.usb: HC died; cleaning up >> usb 1-1: USB disconnect, device number 2 >> >> If you have any idea - let me know, otherwise I'll give up. >> >> If we agree on the direction of these patches, then I can go with a >> single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz >> enabled for da850-evm. > > One last questions, and this probably directed at Sekhar, but what > happens if you modify the OPP for the boards with fixed regulators to > enable all the frequencies but with the only available voltage. Is > there harm is running the processor at a higher voltage than > necessary? I did some quick experiments on a different ARM board and > I saw some changes in power consumption. I would think some power > savings might be better than none, but I don't know if it causes> damage. The OMAP-L138 datasheet mentions two versions of devices in core voltage specification: Variable (1.2V-1.0V) for 375 MHz version Variable (1.3V-1.0V) for 456 MHz version If you have 375 MHz version of device, I do not think you should run at 1.3V. I don't know what "damage" it will cause or how long it takes for any of it to be visible. Keeping that aside, I doubt there will be a lot of power-saving benefit without voltage scaling. Even if you see a slightly lower power number when you reduce frequency, there is also work to be done for the scaling operation itself. Thanks, Sekhar
On 16/04/19 2:07 PM, Bartosz Golaszewski wrote: > If we agree on the direction of these patches, then I can go with a > single enabled OPP for lcdk (456 MHz) and all OPPs up to 375 MHz > enabled for da850-evm. Sounds good to me. Thanks, Sekhar
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts index 26f453dc8370..f29ed9010812 100644 --- a/arch/arm/boot/dts/da850-lcdk.dts +++ b/arch/arm/boot/dts/da850-lcdk.dts @@ -155,12 +155,43 @@ }; }; }; + + cvdd: regulator0 { + compatible = "regulator-fixed"; + regulator-name = "cvdd"; + regulator-min-microvolt = <1300000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + regulator-boot-on; + }; }; &ref_clk { clock-frequency = <24000000>; }; +&cpu { + cpu-supply = <&cvdd>; +}; + +/* LCDK has a fixed CVDD of 1.3V, so only op points >= 300MHz are valid */ + +&opp_100 { + status = "disabled"; +}; + +&opp_200 { + status = "disabled"; +}; + +&opp_375 { + status = "okay"; +}; + +&opp_456 { + status = "okay"; +}; + &pmx_core { status = "okay"; diff --git a/arch/arm/boot/dts/da850-lego-ev3.dts b/arch/arm/boot/dts/da850-lego-ev3.dts index 66fcadf0ba91..553717f84483 100644 --- a/arch/arm/boot/dts/da850-lego-ev3.dts +++ b/arch/arm/boot/dts/da850-lego-ev3.dts @@ -125,6 +125,15 @@ amp-supply = <&>; }; + cvdd: regulator0 { + compatible = "regulator-fixed"; + regulator-name = "cvdd"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-always-on; + regulator-boot-on; + }; + /* * This is a 5V current limiting regulator that is shared by USB, * the sensor (input) ports, the motor (output) ports and the A/DC. @@ -204,6 +213,27 @@ clock-frequency = <24000000>; }; +&cpu { + cpu-supply = <&cvdd>; +}; + +/* since we have a fixed regulator, we can't run at these points */ +&opp_100 { + status = "disabled"; +}; + +&opp_200 { + status = "disabled"; +}; + +/* + * The SoC is actually the 456MHz version, but because of the fixed regulator + * This is the fastest we can go. + */ +&opp_375 { + status = "okay"; +}; + &pmx_core { status = "okay"; diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi index 559659b399d0..ee61d1253b58 100644 --- a/arch/arm/boot/dts/da850.dtsi +++ b/arch/arm/boot/dts/da850.dtsi @@ -20,6 +20,62 @@ reg = <0xc0000000 0x0>; }; + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu: cpu@0 { + compatible = "arm,arm926ej-s"; + device_type = "cpu"; + reg = <0>; + clocks = <&psc0 14>; + operating-points-v2 = <&opp_table>; + }; + }; + + opp_table: opp-table { + compatible = "operating-points-v2"; + + opp_100: opp100-100000000 { + opp-hz = /bits/ 64 <100000000>; + opp-microvolt = <1000000 950000 1050000>; + }; + + opp_200: opp110-200000000 { + opp-hz = /bits/ 64 <200000000>; + opp-microvolt = <1100000 1050000 1160000>; + }; + + opp_300: opp120-300000000 { + opp-hz = /bits/ 64 <300000000>; + opp-microvolt = <1200000 1140000 1320000>; + }; + + /* + * Original silicon was 300MHz max, so higher frequencies + * need to be enabled on a per-board basis if the chip is + * capable. + */ + + opp_375: opp120-375000000 { + status = "disabled"; + opp-hz = /bits/ 64 <375000000>; + opp-microvolt = <1200000 1140000 1320000>; + }; + + opp_415: opp130-415000000 { + status = "disabled"; + opp-hz = /bits/ 64 <415000000>; + opp-microvolt = <1300000 1250000 1350000>; + }; + + opp_456: opp130-456000000 { + status = "disabled"; + opp-hz = /bits/ 64 <456000000>; + opp-microvolt = <1300000 1250000 1350000>; + }; + }; + arm { #address-cells = <1>; #size-cells = <1>;