Message ID | 20191015211618.20758-1-digetx@gmail.com |
---|---|
Headers | show |
Series | NVIDIA Tegra20 CPUFreq driver major update | expand |
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Re-parenting to intermediate clock is supported now by the clock driver > and thus there is no need in a customized CPUFreq driver, all that code > is common for both Tegra20 and Tegra30. The available CPU freqs are now > specified in device-tree in a form of OPPs, all users should update their > device-trees. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/cpufreq/Kconfig.arm | 4 +- > drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- > 3 files changed, 55 insertions(+), 187 deletions(-) > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > index a905796f7f85..2118c45d0acd 100644 > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ > default y > > config ARM_TEGRA20_CPUFREQ > - tristate "Tegra20 CPUFreq support" > - depends on ARCH_TEGRA > + bool "Tegra20 CPUFreq support" Google is currently working on the GKI (generic kernel image) project where they want to use a single kernel image with modules for all kind of android devices. And for that they need all such drivers to be built as module. Since this is already an module, I would ask you to keep it as is instead of moving it to bool here. Else some google guy will switch it back as module later on. LGTM otherwise. Nice work. Thanks.
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Operating Point are specified per HW version. The OPP voltages are kept > in a separate DTSI file because some boards may not define CPU regulator > in their device-tree if voltage scaling isn't necessary, like for example > in a case of tegra20-trimslice which is outlet-powered device. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../boot/dts/tegra20-cpu-opp-microvolt.dtsi | 201 ++++++++++++ > arch/arm/boot/dts/tegra20-cpu-opp.dtsi | 302 ++++++++++++++++++ > 2 files changed, 503 insertions(+) > create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp.dtsi > > diff --git a/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > new file mode 100644 > index 000000000000..e85ffdbef876 > --- /dev/null > +++ b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > @@ -0,0 +1,201 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/ { > + cpu0_opp_table: cpu_opp_table0 { > + opp@216000000_750 { Maybe just drop the _750 (i.e. voltage) from the names as we don't generally follow it :) > + opp-microvolt = <750000 750000 1125000>; > + };
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Hello, > > This series moves intermediate-clk handling from tegra20-cpufreq into > tegra-clk driver, this allows us to switch to generic cpufreq-dt driver > which brings voltage scaling, per-hardware OPPs and Tegra30 support out > of the box. All boards need to adopt CPU OPPs in their device-trees in > order to get cpufreq support. This series adds OPPs only to selective > boards because there is assumption in a current device-trees that CPU > voltage is set for 1GHz freq and this won't work for those CPUs that > can go over 1GHz and thus require voltage regulators to be set up for > voltage scaling support (CC'ed Marcel for Toradex boards). We could > probably add delete-node for OPPs over 1GHz if there are not actively > maintained boards. How do you want to get these patches merged ? Can I just pick the cpufreq bits alone ?
16.10.2019 08:27, Viresh Kumar пишет: > On 16-10-19, 00:16, Dmitry Osipenko wrote: >> Hello, >> >> This series moves intermediate-clk handling from tegra20-cpufreq into >> tegra-clk driver, this allows us to switch to generic cpufreq-dt driver >> which brings voltage scaling, per-hardware OPPs and Tegra30 support out >> of the box. All boards need to adopt CPU OPPs in their device-trees in >> order to get cpufreq support. This series adds OPPs only to selective >> boards because there is assumption in a current device-trees that CPU >> voltage is set for 1GHz freq and this won't work for those CPUs that >> can go over 1GHz and thus require voltage regulators to be set up for >> voltage scaling support (CC'ed Marcel for Toradex boards). We could >> probably add delete-node for OPPs over 1GHz if there are not actively >> maintained boards. > > How do you want to get these patches merged ? Can I just pick the cpufreq bits > alone ? > The cpufreq bits strictly depend on the clk patches and the regulators coupler/balancer series. Hence all patches in this series should collect acks from relevant maintainers and then Thierry will pick up the patchsets in a correct order via tegra tree, at least that's my vision. Thierry, are you okay with that approach?
16.10.2019 08:23, Viresh Kumar пишет: > On 16-10-19, 00:16, Dmitry Osipenko wrote: >> Operating Point are specified per HW version. The OPP voltages are kept >> in a separate DTSI file because some boards may not define CPU regulator >> in their device-tree if voltage scaling isn't necessary, like for example >> in a case of tegra20-trimslice which is outlet-powered device. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> .../boot/dts/tegra20-cpu-opp-microvolt.dtsi | 201 ++++++++++++ >> arch/arm/boot/dts/tegra20-cpu-opp.dtsi | 302 ++++++++++++++++++ >> 2 files changed, 503 insertions(+) >> create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi >> create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp.dtsi >> >> diff --git a/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi >> new file mode 100644 >> index 000000000000..e85ffdbef876 >> --- /dev/null >> +++ b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi >> @@ -0,0 +1,201 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +/ { >> + cpu0_opp_table: cpu_opp_table0 { >> + opp@216000000_750 { > > Maybe just drop the _750 (i.e. voltage) from the names as we don't generally > follow it :) The reason for the _750 postfix is that there are multiple OPPs for 216MHz and they have different voltages for different versions of hardware, thus those are separate OPPs and they can't be squashed into a single OPP node.
16.10.2019 08:18, Viresh Kumar пишет: > On 16-10-19, 00:16, Dmitry Osipenko wrote: >> Re-parenting to intermediate clock is supported now by the clock driver >> and thus there is no need in a customized CPUFreq driver, all that code >> is common for both Tegra20 and Tegra30. The available CPU freqs are now >> specified in device-tree in a form of OPPs, all users should update their >> device-trees. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/cpufreq/Kconfig.arm | 4 +- >> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >> drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- >> 3 files changed, 55 insertions(+), 187 deletions(-) >> >> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm >> index a905796f7f85..2118c45d0acd 100644 >> --- a/drivers/cpufreq/Kconfig.arm >> +++ b/drivers/cpufreq/Kconfig.arm >> @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ >> default y >> >> config ARM_TEGRA20_CPUFREQ >> - tristate "Tegra20 CPUFreq support" >> - depends on ARCH_TEGRA >> + bool "Tegra20 CPUFreq support" > > Google is currently working on the GKI (generic kernel image) project where they > want to use a single kernel image with modules for all kind of android devices. > And for that they need all such drivers to be built as module. Since this is > already an module, I would ask you to keep it as is instead of moving it to bool > here. Else some google guy will switch it back as module later on. > > LGTM otherwise. Nice work. Thanks. > Okay, I'll keep the modularity in v2. Although, tegra20-cpufreq isn't a driver anymore because now it merely prepares OPP table for the cpufreq-dt driver, which is really a one-shot action that is enough to do during boot and thus modularity is a bit redundant here.
On Wed, Oct 16, 2019 at 04:16:27PM +0300, Dmitry Osipenko wrote: > 16.10.2019 08:27, Viresh Kumar пишет: > > On 16-10-19, 00:16, Dmitry Osipenko wrote: > >> Hello, > >> > >> This series moves intermediate-clk handling from tegra20-cpufreq into > >> tegra-clk driver, this allows us to switch to generic cpufreq-dt driver > >> which brings voltage scaling, per-hardware OPPs and Tegra30 support out > >> of the box. All boards need to adopt CPU OPPs in their device-trees in > >> order to get cpufreq support. This series adds OPPs only to selective > >> boards because there is assumption in a current device-trees that CPU > >> voltage is set for 1GHz freq and this won't work for those CPUs that > >> can go over 1GHz and thus require voltage regulators to be set up for > >> voltage scaling support (CC'ed Marcel for Toradex boards). We could > >> probably add delete-node for OPPs over 1GHz if there are not actively > >> maintained boards. > > > > How do you want to get these patches merged ? Can I just pick the cpufreq bits > > alone ? > > > > The cpufreq bits strictly depend on the clk patches and the regulators > coupler/balancer series. Hence all patches in this series should collect > acks from relevant maintainers and then Thierry will pick up the > patchsets in a correct order via tegra tree, at least that's my vision. > > Thierry, are you okay with that approach? Works for me. I already have a set of clock patches that I'd like to merge via the Tegra tree because of a runtime dependency, so it'd be easy to apply these on top of that. Thierry
16.10.2019 17:01, Thierry Reding пишет: > On Wed, Oct 16, 2019 at 04:16:27PM +0300, Dmitry Osipenko wrote: >> 16.10.2019 08:27, Viresh Kumar пишет: >>> On 16-10-19, 00:16, Dmitry Osipenko wrote: >>>> Hello, >>>> >>>> This series moves intermediate-clk handling from tegra20-cpufreq into >>>> tegra-clk driver, this allows us to switch to generic cpufreq-dt driver >>>> which brings voltage scaling, per-hardware OPPs and Tegra30 support out >>>> of the box. All boards need to adopt CPU OPPs in their device-trees in >>>> order to get cpufreq support. This series adds OPPs only to selective >>>> boards because there is assumption in a current device-trees that CPU >>>> voltage is set for 1GHz freq and this won't work for those CPUs that >>>> can go over 1GHz and thus require voltage regulators to be set up for >>>> voltage scaling support (CC'ed Marcel for Toradex boards). We could >>>> probably add delete-node for OPPs over 1GHz if there are not actively >>>> maintained boards. >>> >>> How do you want to get these patches merged ? Can I just pick the cpufreq bits >>> alone ? >>> >> >> The cpufreq bits strictly depend on the clk patches and the regulators >> coupler/balancer series. Hence all patches in this series should collect >> acks from relevant maintainers and then Thierry will pick up the >> patchsets in a correct order via tegra tree, at least that's my vision. >> >> Thierry, are you okay with that approach? > > Works for me. I already have a set of clock patches that I'd like to > merge via the Tegra tree because of a runtime dependency, so it'd be > easy to apply these on top of that. Awesome, thank you very much! Viresh, then only acks to the patches related to cpufreq driver are needed from you for this series.
On Wed, Oct 16, 2019 at 9:29 AM Dmitry Osipenko <digetx@gmail.com> wrote: > > 16.10.2019 08:18, Viresh Kumar пишет: > > On 16-10-19, 00:16, Dmitry Osipenko wrote: > >> Re-parenting to intermediate clock is supported now by the clock driver > >> and thus there is no need in a customized CPUFreq driver, all that code > >> is common for both Tegra20 and Tegra30. The available CPU freqs are now > >> specified in device-tree in a form of OPPs, all users should update their > >> device-trees. > >> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> drivers/cpufreq/Kconfig.arm | 4 +- > >> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > >> drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- > >> 3 files changed, 55 insertions(+), 187 deletions(-) > >> > >> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > >> index a905796f7f85..2118c45d0acd 100644 > >> --- a/drivers/cpufreq/Kconfig.arm > >> +++ b/drivers/cpufreq/Kconfig.arm > >> @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ > >> default y > >> > >> config ARM_TEGRA20_CPUFREQ > >> - tristate "Tegra20 CPUFreq support" > >> - depends on ARCH_TEGRA > >> + bool "Tegra20 CPUFreq support" > > > > Google is currently working on the GKI (generic kernel image) project where they > > want to use a single kernel image with modules for all kind of android devices. > > And for that they need all such drivers to be built as module. Since this is > > already an module, I would ask you to keep it as is instead of moving it to bool > > here. Else some google guy will switch it back as module later on. > > > > LGTM otherwise. Nice work. Thanks. > > > > Okay, I'll keep the modularity in v2. > > Although, tegra20-cpufreq isn't a driver anymore because now it merely > prepares OPP table for the cpufreq-dt driver, which is really a one-shot > action that is enough to do during boot and thus modularity is a bit > redundant here. I doubt Google will care much, since Android has moved on to aarch64. Do they even support arm32 any more?
16.10.2019 17:58, Peter Geis пишет: > On Wed, Oct 16, 2019 at 9:29 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 16.10.2019 08:18, Viresh Kumar пишет: >>> On 16-10-19, 00:16, Dmitry Osipenko wrote: >>>> Re-parenting to intermediate clock is supported now by the clock driver >>>> and thus there is no need in a customized CPUFreq driver, all that code >>>> is common for both Tegra20 and Tegra30. The available CPU freqs are now >>>> specified in device-tree in a form of OPPs, all users should update their >>>> device-trees. >>>> >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>> --- >>>> drivers/cpufreq/Kconfig.arm | 4 +- >>>> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >>>> drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- >>>> 3 files changed, 55 insertions(+), 187 deletions(-) >>>> >>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm >>>> index a905796f7f85..2118c45d0acd 100644 >>>> --- a/drivers/cpufreq/Kconfig.arm >>>> +++ b/drivers/cpufreq/Kconfig.arm >>>> @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ >>>> default y >>>> >>>> config ARM_TEGRA20_CPUFREQ >>>> - tristate "Tegra20 CPUFreq support" >>>> - depends on ARCH_TEGRA >>>> + bool "Tegra20 CPUFreq support" >>> >>> Google is currently working on the GKI (generic kernel image) project where they >>> want to use a single kernel image with modules for all kind of android devices. >>> And for that they need all such drivers to be built as module. Since this is >>> already an module, I would ask you to keep it as is instead of moving it to bool >>> here. Else some google guy will switch it back as module later on. >>> >>> LGTM otherwise. Nice work. Thanks. >>> >> >> Okay, I'll keep the modularity in v2. >> >> Although, tegra20-cpufreq isn't a driver anymore because now it merely >> prepares OPP table for the cpufreq-dt driver, which is really a one-shot >> action that is enough to do during boot and thus modularity is a bit >> redundant here. > > I doubt Google will care much, since Android has moved on to aarch64. > Do they even support arm32 any more? Yes, I don't think there is a real need to care about Google. They won't use pure upstream and won't care about older hardware any ways.
On 16-10-19, 16:21, Dmitry Osipenko wrote: > 16.10.2019 08:23, Viresh Kumar пишет: > > On 16-10-19, 00:16, Dmitry Osipenko wrote: > >> Operating Point are specified per HW version. The OPP voltages are kept > >> in a separate DTSI file because some boards may not define CPU regulator > >> in their device-tree if voltage scaling isn't necessary, like for example > >> in a case of tegra20-trimslice which is outlet-powered device. > >> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> .../boot/dts/tegra20-cpu-opp-microvolt.dtsi | 201 ++++++++++++ > >> arch/arm/boot/dts/tegra20-cpu-opp.dtsi | 302 ++++++++++++++++++ > >> 2 files changed, 503 insertions(+) > >> create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > >> create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp.dtsi > >> > >> diff --git a/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > >> new file mode 100644 > >> index 000000000000..e85ffdbef876 > >> --- /dev/null > >> +++ b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > >> @@ -0,0 +1,201 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > >> + > >> +/ { > >> + cpu0_opp_table: cpu_opp_table0 { > >> + opp@216000000_750 { > > > > Maybe just drop the _750 (i.e. voltage) from the names as we don't generally > > follow it :) > > The reason for the _750 postfix is that there are multiple OPPs for > 216MHz and they have different voltages for different versions of > hardware, thus those are separate OPPs and they can't be squashed into a > single OPP node. Ah, okay. I missed that you are using supported-hw bindings.
On 16-10-19, 21:19, Dmitry Osipenko wrote: > 16.10.2019 17:58, Peter Geis пишет: > > On Wed, Oct 16, 2019 at 9:29 AM Dmitry Osipenko <digetx@gmail.com> wrote: > >> > >> 16.10.2019 08:18, Viresh Kumar пишет: > >>> On 16-10-19, 00:16, Dmitry Osipenko wrote: > >>>> Re-parenting to intermediate clock is supported now by the clock driver > >>>> and thus there is no need in a customized CPUFreq driver, all that code > >>>> is common for both Tegra20 and Tegra30. The available CPU freqs are now > >>>> specified in device-tree in a form of OPPs, all users should update their > >>>> device-trees. > >>>> > >>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >>>> --- > >>>> drivers/cpufreq/Kconfig.arm | 4 +- > >>>> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > >>>> drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- > >>>> 3 files changed, 55 insertions(+), 187 deletions(-) > >>>> > >>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > >>>> index a905796f7f85..2118c45d0acd 100644 > >>>> --- a/drivers/cpufreq/Kconfig.arm > >>>> +++ b/drivers/cpufreq/Kconfig.arm > >>>> @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ > >>>> default y > >>>> > >>>> config ARM_TEGRA20_CPUFREQ > >>>> - tristate "Tegra20 CPUFreq support" > >>>> - depends on ARCH_TEGRA > >>>> + bool "Tegra20 CPUFreq support" > >>> > >>> Google is currently working on the GKI (generic kernel image) project where they > >>> want to use a single kernel image with modules for all kind of android devices. > >>> And for that they need all such drivers to be built as module. Since this is > >>> already an module, I would ask you to keep it as is instead of moving it to bool > >>> here. Else some google guy will switch it back as module later on. > >>> > >>> LGTM otherwise. Nice work. Thanks. > >>> > >> > >> Okay, I'll keep the modularity in v2. > >> > >> Although, tegra20-cpufreq isn't a driver anymore because now it merely > >> prepares OPP table for the cpufreq-dt driver, which is really a one-shot > >> action that is enough to do during boot and thus modularity is a bit > >> redundant here. > > > > I doubt Google will care much, since Android has moved on to aarch64. > > Do they even support arm32 any more? > > Yes, I don't think there is a real need to care about Google. They won't > use pure upstream and won't care about older hardware any ways. Well, using (almost) pure upstream is the idea I believe. And the thing is they want to use a single multi-platform image which should be as small as possible in size. So it won't have any drivers or platform stuff (if possible) and everything is module. I am not sure about arm32/64 thing though. And it is okay if you don't want to care about Google right now. That was just some side knowledge I had :)
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Operating Point are specified per HW version. The OPP voltages are kept > in a separate DTSI file because some boards may not define CPU regulator > in their device-tree if voltage scaling isn't necessary, like for example > in a case of tegra20-trimslice which is outlet-powered device. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../boot/dts/tegra20-cpu-opp-microvolt.dtsi | 201 ++++++++++++ > arch/arm/boot/dts/tegra20-cpu-opp.dtsi | 302 ++++++++++++++++++ > 2 files changed, 503 insertions(+) > create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > create mode 100644 arch/arm/boot/dts/tegra20-cpu-opp.dtsi > > diff --git a/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > new file mode 100644 > index 000000000000..e85ffdbef876 > --- /dev/null > +++ b/arch/arm/boot/dts/tegra20-cpu-opp-microvolt.dtsi > @@ -0,0 +1,201 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/ { > + cpu0_opp_table: cpu_opp_table0 { > + opp@216000000_750 { > + opp-microvolt = <750000 750000 1125000>; > + }; > + > + opp@216000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@312000000_750 { > + opp-microvolt = <750000 750000 1125000>; > + }; > + > + opp@312000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@456000000_750 { > + opp-microvolt = <750000 750000 1125000>; > + }; > + > + opp@456000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@456000000_800_2_2 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@456000000_800_3_2 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@456000000_825 { > + opp-microvolt = <825000 825000 1125000>; > + }; > + > + opp@608000000_750 { > + opp-microvolt = <750000 750000 1125000>; > + }; > + > + opp@608000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@608000000_800_3_2 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@608000000_825 { > + opp-microvolt = <825000 825000 1125000>; > + }; > + > + opp@608000000_850 { > + opp-microvolt = <850000 850000 1125000>; > + }; > + > + opp@608000000_900 { > + opp-microvolt = <900000 900000 1125000>; > + }; > + > + opp@760000000_775 { > + opp-microvolt = <775000 775000 1125000>; > + }; > + > + opp@760000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@760000000_850 { > + opp-microvolt = <850000 850000 1125000>; > + }; > + > + opp@760000000_875 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@760000000_875_1_1 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@760000000_875_0_2 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@760000000_875_1_2 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@760000000_900 { > + opp-microvolt = <900000 900000 1125000>; > + }; > + > + opp@760000000_975 { > + opp-microvolt = <975000 975000 1125000>; > + }; > + > + opp@816000000_800 { > + opp-microvolt = <800000 800000 1125000>; > + }; > + > + opp@816000000_850 { > + opp-microvolt = <850000 850000 1125000>; > + }; > + > + opp@816000000_875 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@816000000_950 { > + opp-microvolt = <950000 950000 1125000>; > + }; > + > + opp@816000000_1000 { > + opp-microvolt = <1000000 1000000 1125000>; > + }; > + > + opp@912000000_850 { > + opp-microvolt = <850000 850000 1125000>; > + }; > + > + opp@912000000_900 { > + opp-microvolt = <900000 900000 1125000>; > + }; > + > + opp@912000000_925 { > + opp-microvolt = <925000 925000 1125000>; > + }; > + > + opp@912000000_950 { > + opp-microvolt = <950000 950000 1125000>; > + }; > + > + opp@912000000_950_0_2 { > + opp-microvolt = <950000 950000 1125000>; > + }; > + > + opp@912000000_950_2_2 { > + opp-microvolt = <950000 950000 1125000>; > + }; > + > + opp@912000000_1000 { > + opp-microvolt = <1000000 1000000 1125000>; > + }; > + > + opp@912000000_1050 { > + opp-microvolt = <1050000 1050000 1125000>; > + }; > + > + opp@1000000000_875 { > + opp-microvolt = <875000 875000 1125000>; > + }; > + > + opp@1000000000_900 { > + opp-microvolt = <900000 900000 1125000>; > + }; > + > + opp@1000000000_950 { > + opp-microvolt = <950000 950000 1125000>; > + }; > + > + opp@1000000000_975 { > + opp-microvolt = <975000 975000 1125000>; > + }; > + > + opp@1000000000_1000 { > + opp-microvolt = <1000000 1000000 1125000>; > + }; > + > + opp@1000000000_1000_0_2 { > + opp-microvolt = <1000000 1000000 1125000>; > + }; > + > + opp@1000000000_1025 { > + opp-microvolt = <1025000 1025000 1125000>; > + }; > + > + opp@1000000000_1100 { > + opp-microvolt = <1100000 1100000 1125000>; > + }; > + > + opp@1200000000_1000 { > + opp-microvolt = <1000000 1000000 1125000>; > + }; > + > + opp@1200000000_1050 { > + opp-microvolt = <1050000 1050000 1125000>; > + }; > + > + opp@1200000000_1100 { > + opp-microvolt = <1100000 1100000 1125000>; > + }; > + > + opp@1200000000_1125 { > + opp-microvolt = <1125000 1125000 1125000>; > + }; > + }; > +}; > diff --git a/arch/arm/boot/dts/tegra20-cpu-opp.dtsi b/arch/arm/boot/dts/tegra20-cpu-opp.dtsi > new file mode 100644 > index 000000000000..c878f4231791 > --- /dev/null > +++ b/arch/arm/boot/dts/tegra20-cpu-opp.dtsi > @@ -0,0 +1,302 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/ { > + cpu0_opp_table: cpu_opp_table0 { > + compatible = "operating-points-v2"; > + opp-shared; > + > + opp@216000000_750 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x0F 0x0003>; > + opp-hz = /bits/ 64 <216000000>; > + }; > + > + opp@216000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x0F 0x0004>; > + opp-hz = /bits/ 64 <216000000>; > + }; > + > + opp@312000000_750 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x0F 0x0003>; > + opp-hz = /bits/ 64 <312000000>; > + }; > + > + opp@312000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x0F 0x0004>; > + opp-hz = /bits/ 64 <312000000>; > + }; > + > + opp@456000000_750 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x0C 0x0003>; > + opp-hz = /bits/ 64 <456000000>; > + }; > + > + opp@456000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0006>; > + opp-hz = /bits/ 64 <456000000>; > + }; > + > + opp@456000000_800_2_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0004>; > + opp-hz = /bits/ 64 <456000000>; > + }; > + > + opp@456000000_800_3_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0004>; > + opp-hz = /bits/ 64 <456000000>; > + }; > + > + opp@456000000_825 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <456000000>; > + }; > + > + opp@608000000_750 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0003>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@608000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0006>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@608000000_800_3_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0004>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@608000000_825 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0001>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@608000000_850 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0006>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@608000000_900 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <608000000>; > + }; > + > + opp@760000000_775 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0003>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0004>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_850 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0006>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_875 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0001>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_875_1_1 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x02 0x0002>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_875_0_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0004>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_875_1_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x02 0x0004>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_900 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0002>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@760000000_975 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <760000000>; > + }; > + > + opp@816000000_800 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0007>; > + opp-hz = /bits/ 64 <816000000>; > + }; > + > + opp@816000000_850 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0002>; > + opp-hz = /bits/ 64 <816000000>; > + }; > + > + opp@816000000_875 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0005>; > + opp-hz = /bits/ 64 <816000000>; > + }; > + > + opp@816000000_950 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0006>; > + opp-hz = /bits/ 64 <816000000>; > + }; > + > + opp@816000000_1000 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <816000000>; > + }; > + > + opp@912000000_850 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0007>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_900 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0002>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_925 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0001>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_950 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x02 0x0006>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_950_0_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0004>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_950_2_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0004>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_1000 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0002>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@912000000_1050 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <912000000>; > + }; > + > + opp@1000000000_875 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0007>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_900 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0002>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_950 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0004>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_975 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0001>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_1000 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x02 0x0006>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_1000_0_2 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0004>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_1025 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0002>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1000000000_1100 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x03 0x0001>; > + opp-hz = /bits/ 64 <1000000000>; > + }; > + > + opp@1200000000_1000 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x08 0x0004>; > + opp-hz = /bits/ 64 <1200000000>; > + }; > + > + opp@1200000000_1050 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x04 0x0004>; > + opp-hz = /bits/ 64 <1200000000>; > + }; > + > + opp@1200000000_1100 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x02 0x0004>; > + opp-hz = /bits/ 64 <1200000000>; > + }; > + > + opp@1200000000_1125 { > + clock-latency-ns = <400000>; > + opp-supported-hw = <0x01 0x0004>; > + opp-hz = /bits/ 64 <1200000000>; > + }; > + }; > +}; Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Operating Point are specified per HW version. The OPP voltages are kept > in a separate DTSI file because some boards may not define CPU regulator > in their device-tree if voltage scaling isn't necessary for them. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../boot/dts/tegra30-cpu-opp-microvolt.dtsi | 801 +++++++++++ > arch/arm/boot/dts/tegra30-cpu-opp.dtsi | 1202 +++++++++++++++++ > 2 files changed, 2003 insertions(+) > create mode 100644 arch/arm/boot/dts/tegra30-cpu-opp-microvolt.dtsi > create mode 100644 arch/arm/boot/dts/tegra30-cpu-opp.dtsi Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 16-10-19, 00:16, Dmitry Osipenko wrote: > Re-parenting to intermediate clock is supported now by the clock driver > and thus there is no need in a customized CPUFreq driver, all that code > is common for both Tegra20 and Tegra30. The available CPU freqs are now > specified in device-tree in a form of OPPs, all users should update their > device-trees. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/cpufreq/Kconfig.arm | 4 +- > drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- > 3 files changed, 55 insertions(+), 187 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
17.10.2019 05:32, Viresh Kumar пишет: > On 16-10-19, 21:19, Dmitry Osipenko wrote: >> 16.10.2019 17:58, Peter Geis пишет: >>> On Wed, Oct 16, 2019 at 9:29 AM Dmitry Osipenko <digetx@gmail.com> wrote: >>>> >>>> 16.10.2019 08:18, Viresh Kumar пишет: >>>>> On 16-10-19, 00:16, Dmitry Osipenko wrote: >>>>>> Re-parenting to intermediate clock is supported now by the clock driver >>>>>> and thus there is no need in a customized CPUFreq driver, all that code >>>>>> is common for both Tegra20 and Tegra30. The available CPU freqs are now >>>>>> specified in device-tree in a form of OPPs, all users should update their >>>>>> device-trees. >>>>>> >>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>>>> --- >>>>>> drivers/cpufreq/Kconfig.arm | 4 +- >>>>>> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >>>>>> drivers/cpufreq/tegra20-cpufreq.c | 236 ++++++--------------------- >>>>>> 3 files changed, 55 insertions(+), 187 deletions(-) >>>>>> >>>>>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm >>>>>> index a905796f7f85..2118c45d0acd 100644 >>>>>> --- a/drivers/cpufreq/Kconfig.arm >>>>>> +++ b/drivers/cpufreq/Kconfig.arm >>>>>> @@ -301,8 +301,8 @@ config ARM_TANGO_CPUFREQ >>>>>> default y >>>>>> >>>>>> config ARM_TEGRA20_CPUFREQ >>>>>> - tristate "Tegra20 CPUFreq support" >>>>>> - depends on ARCH_TEGRA >>>>>> + bool "Tegra20 CPUFreq support" >>>>> >>>>> Google is currently working on the GKI (generic kernel image) project where they >>>>> want to use a single kernel image with modules for all kind of android devices. >>>>> And for that they need all such drivers to be built as module. Since this is >>>>> already an module, I would ask you to keep it as is instead of moving it to bool >>>>> here. Else some google guy will switch it back as module later on. >>>>> >>>>> LGTM otherwise. Nice work. Thanks. >>>>> >>>> >>>> Okay, I'll keep the modularity in v2. >>>> >>>> Although, tegra20-cpufreq isn't a driver anymore because now it merely >>>> prepares OPP table for the cpufreq-dt driver, which is really a one-shot >>>> action that is enough to do during boot and thus modularity is a bit >>>> redundant here. >>> >>> I doubt Google will care much, since Android has moved on to aarch64. >>> Do they even support arm32 any more? >> >> Yes, I don't think there is a real need to care about Google. They won't >> use pure upstream and won't care about older hardware any ways. > > Well, using (almost) pure upstream is the idea I believe. And the thing is they > want to use a single multi-platform image which should be as small as possible > in size. So it won't have any drivers or platform stuff (if possible) and > everything is module. > > I am not sure about arm32/64 thing though. And it is okay if you don't want to > care about Google right now. That was just some side knowledge I had :) > I'll leave the module part as-is for now.
On Wed, Oct 16, 2019 at 12:16:02AM +0300, Dmitry Osipenko wrote: > CCLK stands for "CPU Clock", CPU core is running off CCLK. CCLK supports > multiple parents and it has internal clock divider which uses clock > skipping technique, meaning that CPU's voltage should correspond to the > parent clock rate and not CCLK. PLLX is the main CCLK parent that provides > clock rates above 1GHz and it has special property such that the CCLK's > internal divider is set into bypass mode when PLLX is set as a parent for > CCLK. > > This patch forks generic Super Clock into CCLK implementation which takes > into account all CCLK specifics. The proper CCLK implementation is needed > by the upcoming Tegra20 CPUFreq driver update that will allow to utilize > the generic cpufreq-dt driver by moving intermediate clock handling into > the clock driver. Note that technically this all could be squashed into > clk-super, but result will be messier. > > Note that currently all CCLKLP bits are left in the clk-super.c and only > CCLKG is supported by clk-tegra-super-cclk. It shouldn't be difficult > to move the CCLKLP bits, but CCLKLP is not used by anything in kernel > and thus better not to touch it for now. .. > + super->reg = reg; > + super->lock = lock; > + super->width = 4; > + super->flags = clk_super_flags; > + super->frac_div.reg = reg + 4; > + super->frac_div.shift = 16; > + super->frac_div.width = 8; > + super->frac_div.frac_width = 1; > + super->frac_div.lock = lock; > + super->frac_div.flags = TEGRA_DIVIDER_SUPER; > + super->div_ops = &tegra_clk_frac_div_ops; > + This is not right. The super clock divider is not a divider, it's a pulse skipper. > + /* Data in .init is copied by clk_register(), so stack variable OK */ > + super->hw.init = &init; > + > + clk = clk_register(NULL, &super->hw); > + if (IS_ERR(clk)) > + kfree(super); > + > + return clk; > +} > diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h > index f81c10654aa9..095595a5b8a8 100644 > --- a/drivers/clk/tegra/clk.h > +++ b/drivers/clk/tegra/clk.h > @@ -699,6 +699,10 @@ struct clk *tegra_clk_register_super_clk(const char *name, > const char * const *parent_names, u8 num_parents, > unsigned long flags, void __iomem *reg, u8 clk_super_flags, > spinlock_t *lock); > +struct clk *tegra_clk_register_super_cclk(const char *name, > + const char * const *parent_names, u8 num_parents, > + unsigned long flags, void __iomem *reg, u8 clk_super_flags, > + spinlock_t *lock); > > /** > * struct tegra_sdmmc_mux - switch divider with Low Jitter inputs for SDMMC > -- > 2.23.0 >
28.10.2019 17:57, Peter De Schrijver пишет: > On Wed, Oct 16, 2019 at 12:16:02AM +0300, Dmitry Osipenko wrote: >> CCLK stands for "CPU Clock", CPU core is running off CCLK. CCLK supports >> multiple parents and it has internal clock divider which uses clock >> skipping technique, meaning that CPU's voltage should correspond to the >> parent clock rate and not CCLK. PLLX is the main CCLK parent that provides >> clock rates above 1GHz and it has special property such that the CCLK's >> internal divider is set into bypass mode when PLLX is set as a parent for >> CCLK. >> >> This patch forks generic Super Clock into CCLK implementation which takes >> into account all CCLK specifics. The proper CCLK implementation is needed >> by the upcoming Tegra20 CPUFreq driver update that will allow to utilize >> the generic cpufreq-dt driver by moving intermediate clock handling into >> the clock driver. Note that technically this all could be squashed into >> clk-super, but result will be messier. >> >> Note that currently all CCLKLP bits are left in the clk-super.c and only >> CCLKG is supported by clk-tegra-super-cclk. It shouldn't be difficult >> to move the CCLKLP bits, but CCLKLP is not used by anything in kernel >> and thus better not to touch it for now. > > .. > >> + super->reg = reg; >> + super->lock = lock; >> + super->width = 4; >> + super->flags = clk_super_flags; >> + super->frac_div.reg = reg + 4; >> + super->frac_div.shift = 16; >> + super->frac_div.width = 8; >> + super->frac_div.frac_width = 1; >> + super->frac_div.lock = lock; >> + super->frac_div.flags = TEGRA_DIVIDER_SUPER; >> + super->div_ops = &tegra_clk_frac_div_ops; >> + > > This is not right. The super clock divider is not a divider, it's a > pulse skipper. For the reference: on #tegra Peter explained to me in a more details what was meant here. Turned out that T30+ has a real CCLK divider and we won't use the pulse skipper for T20 nor for T30+, I'll update clk patches accordingly in the next revision.