| Message ID | 20200619082148.4qyv6wt2fmt7s44h@SvensMacbookPro.hq.voleatech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1,v2] cpufreq: ap806: fix cpufreq driver needs ap cpu clk | expand |
Hi Sven, Sven Auhagen <sven.auhagen@voleatech.de> wrote on Fri, 19 Jun 2020 10:21:48 +0200: > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > to work. This dependency is currently not satisfied and > the ARMADA_AP_CPU_CLK can not be selected independently. > > Add it to the mvebu platform the same way the Armada 37XX CLK > driver is enabled on the platform. > > v2: > * Resubmission without email footer The changelog should go after the three dashes... > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > --- Here! Otherwise it would appear as part of your commit message. > arch/arm64/Kconfig.platforms | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index 8dd05b2a925c..efb5355c9eea 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -146,6 +146,7 @@ config ARCH_MVEBU > bool "Marvell EBU SoC Family" > select ARMADA_AP806_SYSCON > select ARMADA_CP110_SYSCON > + select ARMADA_AP_CPU_CLK > select ARMADA_37XX_CLK > select GPIOLIB > select GPIOLIB_IRQCHIP Perhaps this is a fix and would deserve a Fixes tag too. Thanks, Miquèl
Hi Miquèl, On Mon, Jun 22, 2020 at 08:46:35AM +0200, Miquel Raynal wrote: > Hi Sven, > > Sven Auhagen <sven.auhagen@voleatech.de> wrote on Fri, 19 Jun 2020 > 10:21:48 +0200: > > > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > > to work. This dependency is currently not satisfied and > > the ARMADA_AP_CPU_CLK can not be selected independently. > > > > Add it to the mvebu platform the same way the Armada 37XX CLK > > driver is enabled on the platform. > > > > v2: > > * Resubmission without email footer > > The changelog should go after the three dashes... > > > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > --- > > Here! > > Otherwise it would appear as part of your commit message. > Thanks, I will fix it. > > arch/arm64/Kconfig.platforms | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > index 8dd05b2a925c..efb5355c9eea 100644 > > --- a/arch/arm64/Kconfig.platforms > > +++ b/arch/arm64/Kconfig.platforms > > @@ -146,6 +146,7 @@ config ARCH_MVEBU > > bool "Marvell EBU SoC Family" > > select ARMADA_AP806_SYSCON > > select ARMADA_CP110_SYSCON > > + select ARMADA_AP_CPU_CLK > > select ARMADA_37XX_CLK > > select GPIOLIB > > select GPIOLIB_IRQCHIP > > Perhaps this is a fix and would deserve a Fixes tag too. I can add the Fixes tag to the original cpufreq commit for Armada 8k? > > Thanks, > Miquèl Best Sven
On 19-06-20, 10:21, Sven Auhagen wrote: > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > to work. This dependency is currently not satisfied and > the ARMADA_AP_CPU_CLK can not be selected independently. > > Add it to the mvebu platform the same way the Armada 37XX CLK > driver is enabled on the platform. > > v2: > * Resubmission without email footer > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > --- > arch/arm64/Kconfig.platforms | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > index 8dd05b2a925c..efb5355c9eea 100644 > --- a/arch/arm64/Kconfig.platforms > +++ b/arch/arm64/Kconfig.platforms > @@ -146,6 +146,7 @@ config ARCH_MVEBU > bool "Marvell EBU SoC Family" > select ARMADA_AP806_SYSCON > select ARMADA_CP110_SYSCON > + select ARMADA_AP_CPU_CLK What about adding a depends on in the cpufreq Kconfig for the driver instead ?
On Mon, Jun 22, 2020 at 02:39:16PM +0530, Viresh Kumar wrote: > On 19-06-20, 10:21, Sven Auhagen wrote: > > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > > to work. This dependency is currently not satisfied and > > the ARMADA_AP_CPU_CLK can not be selected independently. > > > > Add it to the mvebu platform the same way the Armada 37XX CLK > > driver is enabled on the platform. > > > > v2: > > * Resubmission without email footer > > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > --- > > arch/arm64/Kconfig.platforms | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > index 8dd05b2a925c..efb5355c9eea 100644 > > --- a/arch/arm64/Kconfig.platforms > > +++ b/arch/arm64/Kconfig.platforms > > @@ -146,6 +146,7 @@ config ARCH_MVEBU > > bool "Marvell EBU SoC Family" > > select ARMADA_AP806_SYSCON > > select ARMADA_CP110_SYSCON > > + select ARMADA_AP_CPU_CLK > > What about adding a depends on in the cpufreq Kconfig for the driver instead ? It is also an option. I went with this implementation because it was done the same way with the Armada 37XX and therefore it is more consistent. Best Sven > > -- > viresh
On 22-06-20, 11:17, Sven Auhagen wrote: > On Mon, Jun 22, 2020 at 02:39:16PM +0530, Viresh Kumar wrote: > > On 19-06-20, 10:21, Sven Auhagen wrote: > > > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > > > to work. This dependency is currently not satisfied and > > > the ARMADA_AP_CPU_CLK can not be selected independently. > > > > > > Add it to the mvebu platform the same way the Armada 37XX CLK > > > driver is enabled on the platform. > > > > > > v2: > > > * Resubmission without email footer > > > > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > > --- > > > arch/arm64/Kconfig.platforms | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > > index 8dd05b2a925c..efb5355c9eea 100644 > > > --- a/arch/arm64/Kconfig.platforms > > > +++ b/arch/arm64/Kconfig.platforms > > > @@ -146,6 +146,7 @@ config ARCH_MVEBU > > > bool "Marvell EBU SoC Family" > > > select ARMADA_AP806_SYSCON > > > select ARMADA_CP110_SYSCON > > > + select ARMADA_AP_CPU_CLK > > > > What about adding a depends on in the cpufreq Kconfig for the driver instead ? > > It is also an option. I went with this implementation because it was done the > same way with the Armada 37XX and therefore it is more consistent. I think that depends on is better as it lays out explicitly why the clk is required. i.e. if cpufreq driver isn't compiled then the clk isn't needed. It is far more readable then doing a select somewhere. I will rather break consistency here for the sake of doing the right thing, though we can convert earlier ones to do it properly as well.
On Mon, Jun 22, 2020 at 02:50:26PM +0530, Viresh Kumar wrote: > On 22-06-20, 11:17, Sven Auhagen wrote: > > On Mon, Jun 22, 2020 at 02:39:16PM +0530, Viresh Kumar wrote: > > > On 19-06-20, 10:21, Sven Auhagen wrote: > > > > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > > > > to work. This dependency is currently not satisfied and > > > > the ARMADA_AP_CPU_CLK can not be selected independently. > > > > > > > > Add it to the mvebu platform the same way the Armada 37XX CLK > > > > driver is enabled on the platform. > > > > > > > > v2: > > > > * Resubmission without email footer > > > > > > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > > > --- > > > > arch/arm64/Kconfig.platforms | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > > > index 8dd05b2a925c..efb5355c9eea 100644 > > > > --- a/arch/arm64/Kconfig.platforms > > > > +++ b/arch/arm64/Kconfig.platforms > > > > @@ -146,6 +146,7 @@ config ARCH_MVEBU > > > > bool "Marvell EBU SoC Family" > > > > select ARMADA_AP806_SYSCON > > > > select ARMADA_CP110_SYSCON > > > > + select ARMADA_AP_CPU_CLK > > > > > > What about adding a depends on in the cpufreq Kconfig for the driver instead ? > > > > It is also an option. I went with this implementation because it was done the > > same way with the Armada 37XX and therefore it is more consistent. > > I think that depends on is better as it lays out explicitly why the clk is > required. i.e. if cpufreq driver isn't compiled then the clk isn't needed. It is > far more readable then doing a select somewhere. > > I will rather break consistency here for the sake of doing the right thing, > though we can convert earlier ones to do it properly as well. No problem, let me move it to the cpufreq driver and I will resubmit the patch. Best Sven > > -- > viresh
On 22-06-20, 11:33, Sven Auhagen wrote: > On Mon, Jun 22, 2020 at 02:50:26PM +0530, Viresh Kumar wrote: > > On 22-06-20, 11:17, Sven Auhagen wrote: > > > On Mon, Jun 22, 2020 at 02:39:16PM +0530, Viresh Kumar wrote: > > > > On 19-06-20, 10:21, Sven Auhagen wrote: > > > > > The Armada 8K cpufreq driver needs the Armada AP CPU CLK > > > > > to work. This dependency is currently not satisfied and > > > > > the ARMADA_AP_CPU_CLK can not be selected independently. > > > > > > > > > > Add it to the mvebu platform the same way the Armada 37XX CLK > > > > > driver is enabled on the platform. > > > > > > > > > > v2: > > > > > * Resubmission without email footer > > > > > > > > > > Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> > > > > > --- > > > > > arch/arm64/Kconfig.platforms | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms > > > > > index 8dd05b2a925c..efb5355c9eea 100644 > > > > > --- a/arch/arm64/Kconfig.platforms > > > > > +++ b/arch/arm64/Kconfig.platforms > > > > > @@ -146,6 +146,7 @@ config ARCH_MVEBU > > > > > bool "Marvell EBU SoC Family" > > > > > select ARMADA_AP806_SYSCON > > > > > select ARMADA_CP110_SYSCON > > > > > + select ARMADA_AP_CPU_CLK > > > > > > > > What about adding a depends on in the cpufreq Kconfig for the driver instead ? > > > > > > It is also an option. I went with this implementation because it was done the > > > same way with the Armada 37XX and therefore it is more consistent. > > > > I think that depends on is better as it lays out explicitly why the clk is > > required. i.e. if cpufreq driver isn't compiled then the clk isn't needed. It is > > far more readable then doing a select somewhere. > > > > I will rather break consistency here for the sake of doing the right thing, > > though we can convert earlier ones to do it properly as well. > > No problem, let me move it to the cpufreq driver and I will resubmit the patch. Thanks, and so you can add the Fixes tag as well now :)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index 8dd05b2a925c..efb5355c9eea 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -146,6 +146,7 @@ config ARCH_MVEBU bool "Marvell EBU SoC Family" select ARMADA_AP806_SYSCON select ARMADA_CP110_SYSCON + select ARMADA_AP_CPU_CLK select ARMADA_37XX_CLK select GPIOLIB select GPIOLIB_IRQCHIP
The Armada 8K cpufreq driver needs the Armada AP CPU CLK to work. This dependency is currently not satisfied and the ARMADA_AP_CPU_CLK can not be selected independently. Add it to the mvebu platform the same way the Armada 37XX CLK driver is enabled on the platform. v2: * Resubmission without email footer Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de> --- arch/arm64/Kconfig.platforms | 1 + 1 file changed, 1 insertion(+)