[Skiboot] [PATCH 2/5] NX: Organize NX compression code to include 842 and gzip engines support

[Sukadev Bhattiprolu]

Haren Myneni [haren at linux.vnet.ibm.com] wrote:
> 
> In addition to 842 compression, P9 NX also supports gzip compression.
> So this patch creates nx-compress.c and reorg nx-842.c code so that
> common functions that are needed for both 842 and gzip compression
> will be moved in to nx-compress.c. This patch does not change the
> actual functionality.
> 
> Signed-off-by: Haren Myneni <haren at us.ibm.com>
> ---
>  hw/Makefile.inc  |    2 +-
>  hw/nx-842.c      |   21 +++++++--------------
>  hw/nx-compress.c |   34 ++++++++++++++++++++++++++++++++++
>  hw/nx.c          |    2 +-
>  include/nx.h     |    4 +++-
>  5 files changed, 46 insertions(+), 17 deletions(-)
>  create mode 100644 hw/nx-compress.c
> 
> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
> index f2dc328..97c909e 100644
> --- a/hw/Makefile.inc
> +++ b/hw/Makefile.inc
> @@ -2,7 +2,7 @@
>  SUBDIRS += hw
>  HW_OBJS  = xscom.o chiptod.o gx.o cec.o lpc.o lpc-uart.o psi.o
>  HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o
> -HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-842.o
> +HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-compress.o nx-842.o
>  HW_OBJS += p7ioc.o p7ioc-inits.o p7ioc-phb.o
>  HW_OBJS += phb3.o sfc-ctrl.o fake-rtc.o bt.o p8-i2c.o prd.o
>  HW_OBJS += dts.o lpc-rtc.o npu.o npu-hw-procedures.o xive.o phb4.o
> diff --git a/hw/nx-842.c b/hw/nx-842.c
> index f462f7a..3ccb549 100644
> --- a/hw/nx-842.c
> +++ b/hw/nx-842.c
> @@ -90,7 +90,7 @@ static int nx_cfg_842(u32 gcid, u64 xcfg)
>  	return rc;
>  }
> 
> -static int nx_cfg_dma(u32 gcid, u64 xcfg)
> +static int nx_cfg_842_dma(u32 gcid, u64 xcfg)
>  {
>  	u64 cfg;
>  	int rc;
> @@ -100,9 +100,9 @@ static int nx_cfg_dma(u32 gcid, u64 xcfg)
>  		return rc;
> 
>  	if (proc_gen == proc_gen_p8) {
> -		cfg = SETFIELD(NX_P8_DMA_CFG_842_COMPRESS_PREFETCH, cfg,
> +		cfg = SETFIELD(NX_DMA_CFG_842_COMPRESS_PREFETCH, cfg,
>  			       DMA_COMPRESS_PREFETCH);

If this is just a boolean field, how about s/DMA_COMPRESS_PREFETCH/true/?

> -		cfg = SETFIELD(NX_P8_DMA_CFG_842_DECOMPRESS_PREFETCH, cfg,
> +		cfg = SETFIELD(NX_DMA_CFG_842_DECOMPRESS_PREFETCH, cfg,
>  			       DMA_DECOMPRESS_PREFETCH);

ditto

Also, can you move this change into previous patch to make sure that patch
is bisect safe? It is unrelated to the code reorg anyway.

>  	}
> 
> @@ -131,7 +131,7 @@ static int nx_cfg_dma(u32 gcid, u64 xcfg)
>  	return rc;
>  }
> 
> -static int nx_cfg_ee(u32 gcid, u64 xcfg)
> +static int nx_cfg_842_ee(u32 gcid, u64 xcfg)
>  {
>  	u64 cfg;
>  	int rc;
> @@ -153,18 +153,11 @@ static int nx_cfg_ee(u32 gcid, u64 xcfg)
>  	return rc;
>  }
> 
> -void nx_create_842_node(struct dt_node *node)
> +void nx_enable_842(struct dt_node *node, u32 gcid, u32 pb_base)

I understand parameters are different, but the functionality is still
similar to the other nx_create*_node() functions (i.e it creates the
DT properties) How about leaving the name as is (nx_create_842_node())?

>  {
> -	u32 gcid;
> -	u32 pb_base;
>  	u64 cfg_dma, cfg_842, cfg_ee;
>  	int rc;
> 
> -	gcid = dt_get_chip_id(node);
> -	pb_base = dt_get_address(node, 0, NULL);
> -
> -	prlog(PR_INFO, "NX%d: 842 at 0x%x\n", gcid, pb_base);
> -
>  	if (dt_node_is_compatible(node, "ibm,power7-nx")) {
>  		cfg_dma = pb_base + NX_P7_DMA_CFG;
>  		cfg_842 = pb_base + NX_P7_842_CFG;
> @@ -178,7 +171,7 @@ void nx_create_842_node(struct dt_node *node)
>  		return;
>  	}
> 
> -	rc = nx_cfg_dma(gcid, cfg_dma);
> +	rc = nx_cfg_842_dma(gcid, cfg_dma);
>  	if (rc)
>  		return;
> 
> @@ -186,7 +179,7 @@ void nx_create_842_node(struct dt_node *node)
>  	if (rc)
>  		return;
> 
> -	rc = nx_cfg_ee(gcid, cfg_ee);
> +	rc = nx_cfg_842_ee(gcid, cfg_ee);
>  	if (rc)
>  		return;
> 
> diff --git a/hw/nx-compress.c b/hw/nx-compress.c
> new file mode 100644
> index 0000000..2ea2734
> --- /dev/null
> +++ b/hw/nx-compress.c
> @@ -0,0 +1,34 @@
> +/* Copyright 2015 IBM Corp.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at
> + *
> + * 	http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> + * implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <skiboot.h>
> +#include <chip.h>
> +#include <xscom.h>
> +#include <io.h>
> +#include <cpu.h>
> +#include <nx.h>
> +
> +void nx_create_compress_node(struct dt_node *node)

Since this will create 842 and GZIP nodes in the follow on patch, maybe
call it nx_create_compress_nodes()?

> +{
> +	u32 gcid, pb_base;
> +
> +	gcid = dt_get_chip_id(node);
> +	pb_base = dt_get_address(node, 0, NULL);
> +
> +	prlog(PR_INFO, "NX%d: 842 at 0x%x\n", gcid, pb_base);
> +
> +	nx_enable_842(node, gcid, pb_base);
> +}
> diff --git a/hw/nx.c b/hw/nx.c
> index 83528d1..020c9e4 100644
> --- a/hw/nx.c
> +++ b/hw/nx.c
> @@ -29,6 +29,6 @@ void nx_init(void)
>  	dt_for_each_compatible(dt_root, node, "ibm,power-nx") {
>  		nx_create_rng_node(node);
>  		nx_create_crypto_node(node);
> -		nx_create_842_node(node);
> +		nx_create_compress_node(node);
>  	}
>  }
> diff --git a/include/nx.h b/include/nx.h
> index 4a67020..0a7b1b0 100644
> --- a/include/nx.h
> +++ b/include/nx.h
> @@ -378,7 +378,9 @@
> 
>  extern void nx_create_rng_node(struct dt_node *);
>  extern void nx_create_crypto_node(struct dt_node *);
> -extern void nx_create_842_node(struct dt_node *);
> +extern void nx_create_compress_node(struct dt_node *);
> +
> +extern void nx_enable_842(struct dt_node *, u32 gcid, u32 pb_base);
> 
>  extern void nx_init(void);
> 
> -- 
> 1.7.1
> 
>