Re: [PATCH 12/16] cpuidle: mvebu: Rename the driver from armada-370-xp to mvebu-v7

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Gregory,

On Fri, 27 Jun 2014 15:22:53 +0200, Gregory CLEMENT wrote:
> Actually this driver will be able to manage the cpuidle for more SoCs
> that Armada 370 ad XP. It will support Armada 38x and potentially
> Armada 375. This patch change the names accordingly to this behavior.

I think the last sentence should rather be something like: "This patch
renames the driver as well as the functions and variables used in the
driver".

I would also specifically mention the renaming of the driver that
requires changing the pmsu.c file. It is worth mentioning that this
rename, touching both drivers/cpuidle and arch/arm/mach-mvebu, may
require some special handling in terms of patch merging (the patch
mainly touches drivers/cpuidle so it should theoretically go through
the cpuidle maintainer, but since the pmsu.c file is touched a lot by
other patches, there will probably be lots of conflicts if this patch
goes through the cpuidle tree). Surely something to mention as a
comment in the patch, maybe to get the Acked-by of the cpuidle people
and merge things through the mvebu and arm-soc trees.

> -config ARM_ARMADA_370_XP_CPUIDLE
> -	bool "CPU Idle Driver for Armada 370/XP family processors"
> +config ARM_MVEBU_V7_CPUIDLE
> +	bool "CPU Idle Driver for mvebu v7 family processors"

Actually, what worries me a bit of that Dove is a ARMv7 processor of
the mvebu family, but which is not using this cpuidle driver. Do you
expect Dove to be able to use this driver in the future?

>  	depends on ARCH_MVEBU
>  	help
> -	  Select this to enable cpuidle on Armada 370/XP processors.
> +	  Select this to enable cpuidle on Armada 370, 385 and XP processors.

385 -> 38x, because 380 and 385 are both capable of using this driver.

> diff --git a/drivers/cpuidle/cpuidle-armada-370-xp.c b/drivers/cpuidle/cpuidle-armada-370-xp.c
> deleted file mode 100644
> index a5fba0287bfb..000000000000
> --- a/drivers/cpuidle/cpuidle-armada-370-xp.c
> +++ /dev/null

You haven't enabled rename detection in git? Or git isn't detecting the
rename because of the number of changes in the file?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com