[U-Boot] [PATCH v2 1/2] drivers/misc: Share qbman init between archs

[York Sun]

On 12/08/2017 04:46 PM, Ahmed Mansour wrote:
> This patch adds changes necessary to move functionality present in
> PowerPC folders with ARM architectures that have DPAA1 QBMan hardware
> 
> - Create new board/freescale/common/fsl_portals.c to house shared
>   device tree fixups for DPAA1 devices with ARM and PowerPC cores
> - Add new header file to top includes directory to allow files in
>   both architectures to grab the function prototypes
> - Port inhibit_portals() from PowerPC to ARM. This function is used in
>   setup to disable interrupts on all QMan and BMan portals. It is
>   needed because the interrupts are enabled by default for all portals
>   including unused/uninitialised portals. When the kernel attempts to
>   go to deep sleep the unused portals prevent it from doing so
> 
> Signed-off-by: Ahmed Mansour <ahmed.mansour at nxp.com>
> 
> ---
> 
> Changes in v2:
> - Add get_qman_freq() to replace get_sys_info() for readability
> - Correct the copyright year in new files
> - Replace !ARM with PPC to wall off PowerPC SOCs specific qman setup
> - Rename portals.c -> fsl_portals.c for clarity
> 
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c            |   4 +
>  arch/arm/cpu/armv8/fsl-layerscape/fdt.c            |   9 +
>  .../arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c |  11 +
>  .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  29 ++
>  arch/arm/include/asm/arch-fsl-layerscape/speed.h   |   1 +
>  arch/powerpc/cpu/mpc85xx/cpu_init.c                |   3 +-
>  arch/powerpc/cpu/mpc85xx/fdt.c                     |   1 +
>  arch/powerpc/cpu/mpc85xx/portals.c                 | 281 -------------------
>  arch/powerpc/include/asm/fsl_liodn.h               |   7 +-
>  arch/powerpc/include/asm/fsl_portals.h             |   4 -
>  arch/powerpc/include/asm/immap_85xx.h              |  60 ----
>  drivers/misc/Makefile                              |   1 +
>  drivers/misc/fsl_portals.c                         | 304 +++++++++++++++++++++
>  include/configs/ls1043a_common.h                   |   2 +
>  include/fsl_qbman.h                                |  75 +++++
>  15 files changed, 444 insertions(+), 348 deletions(-)
>  create mode 100644 drivers/misc/fsl_portals.c
>  create mode 100644 include/fsl_qbman.h
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> index d082629..3fd352f 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> @@ -30,6 +30,7 @@
>  #endif
>  #include <asm/arch/clock.h>
>  #include <hwconfig.h>
> +#include <fsl_qbman.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -565,6 +566,9 @@ int arch_early_init_r(void)
>  #ifdef CONFIG_FMAN_ENET
>  	fman_enet_init();
>  #endif
> +#ifdef CONFIG_SYS_DPAA_QBMAN
> +	setup_qbman_portals();
> +#endif
>  	return 0;
>  }
>  
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> index cae59da..382bf3d 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> @@ -26,6 +26,8 @@
>  #ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
>  #include <asm/armv8/sec_firmware.h>
>  #endif
> +#include <asm/arch/speed.h>
> +#include <fsl_qbman.h>
>  
>  int fdt_fixup_phy_connection(void *blob, int offset, phy_interface_t phyc)
>  {
> @@ -415,6 +417,13 @@ void ft_cpu_setup(void *blob, bd_t *bd)
>  	fdt_fixup_esdhc(blob, bd);
>  #endif
>  
> +#ifdef CONFIG_SYS_DPAA_QBMAN
> +	fdt_fixup_bportals(blob);
> +	fdt_fixup_qportals(blob);
> +	do_fixup_by_compat_u32(blob, "fsl,qman",
> +			       "clock-frequency", get_qman_freq(), 1);
> +#endif
> +
>  #ifdef CONFIG_SYS_DPAA_FMAN
>  	fdt_fixup_fman_firmware(blob);
>  #endif
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> index 2d7775e..f707205 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c
> @@ -155,6 +155,17 @@ void get_sys_info(struct sys_info *sys_info)
>  	sys_info->freq_localbus = sys_info->freq_systembus /
>  						CONFIG_SYS_FSL_IFC_CLK_DIV;
>  #endif
> +#ifdef CONFIG_SYS_DPAA_QBMAN
> +	sys_info->freq_qman = sys_info->freq_systembus;
> +#endif
> +}
> +
> +unsigned long get_qman_freq(void)
> +{
> +	struct sys_info sys_info;
> +
> +	get_sys_info(&sys_info);

nitpick
Insert a blank line before return.

> +	return sys_info.freq_systembus;
>  }
> 

If you think this clock will be used multiple times, you can consider to
use the same way sdhc_clk is handled. For this patch, I only see one
place it is called.

<snip>

> diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c
> new file mode 100644
> index 0000000..8f8503f
> --- /dev/null
> +++ b/drivers/misc/fsl_portals.c
> @@ -0,0 +1,304 @@
> +/*
> + * Copyright 2017 NXP

Shouldn't we carry the old license year and copyright for Freescale
because this file is created by moving those code here? I am not a
lawyer though.

Let's take a look from a different angle. I know you use patman to
generate the patches. Sometimes we need to think what the best way is.
Patman is a good tool. But I don't think it turns on the switch
"--find-copies". This switch can detect copies and renames. As you
stated in the change log, you renamed portals.c to fsl_portals.c. Your
patch didn't show this. You can manually run "git format-patch -M -C" to
generate the patches. For this case, the "-C" switch makes a difference.
It detects the renaming so the patch has less insertions, and easier to
review.

15 files changed, 198 insertions(+), 458 deletions(-)

York