[Skiboot] [PATCH 2/5] NX: Organize NX compression code to include 842 and gzip engines support
Haren Myneni
haren at linux.vnet.ibm.com
Wed Feb 22 09:22:20 AEDT 2017
On 02/21/2017 12:03 PM, Sukadev Bhattiprolu wrote:
> 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/?
I was following the same syntax with the existing code.
>
>> - 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())?
Also thought so. nx_create_*_node() functions are called from nx.c (top_level) for different functionalities (compress and encryption). These functions also enable 842 / gzip engines. Since parameters are different, used nx_enable*()
>
>> {
>> - 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()?
NX provides compression, encryption and etc. In each functionality, several engines can be defined / sopported. So I think *compress_node() naming convention should be similar to *encrypt_node().
>
>> +{
>> + 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
>>
>>
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
>
More information about the Skiboot
mailing list