DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
Cc: santosh.shukla@caviumnetworks.com, thomas@monjalon.net,
	anatoly.burakov@intel.com, lironh@marvell.com,
	bruce.richardson@intel.com, fiona.trahe@intel.com,
	shreyansh.jain@nxp.com, hemant.agrawal@nxp.com, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 2/2] octeontx: move mbox to common folder
Date: Wed, 4 Apr 2018 08:59:19 +0530	[thread overview]
Message-ID: <20180404032919.GB8574@jerin> (raw)
In-Reply-To: <20180402091000.9208-2-pbhagavatula@caviumnetworks.com>

-----Original Message-----
> Date: Mon,  2 Apr 2018 14:40:00 +0530
> From: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> To: jerin.jacob@caviumnetworks.com, santosh.shukla@caviumnetworks.com,
>  thomas@monjalon.net, anatoly.burakov@intel.com, lironh@marvell.com,
>  bruce.richardson@intel.com, fiona.trahe@intel.com, shreyansh.jain@nxp.com,
>  hemant.agrawal@nxp.com
> Cc: dev@dpdk.org, Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v4 2/2] octeontx: move mbox to common folder
> X-Mailer: git-send-email 2.16.3
> 
> Move commonly used functions across mempool, event and net devices to a
> common folder in drivers.
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> ---
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/common/octeontx/meson.build b/drivers/common/octeontx/meson.build
> new file mode 100644
> index 000000000..8a28ce800
> --- /dev/null
> +++ b/drivers/common/octeontx/meson.build
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Cavium, Inc
> +#
> +
> +sources = files('octeontx_mbox.c'
> +)

Does it make sense to change to,
sources = files('octeontx_mbox.c')
i.e no sepreate line for ")"

> diff --git a/drivers/mempool/octeontx/octeontx_mbox.c b/drivers/common/octeontx/octeontx_mbox.c
> similarity index 83%
> rename from drivers/mempool/octeontx/octeontx_mbox.c
> rename to drivers/common/octeontx/octeontx_mbox.c
> index f8cb6a453..c98e110f3 100644
> --- a/drivers/mempool/octeontx/octeontx_mbox.c
> +++ b/drivers/common/octeontx/octeontx_mbox.c
> @@ -11,7 +11,6 @@
>  #include <rte_spinlock.h>
> 
>  #include "octeontx_mbox.h"
> -#include "octeontx_pool_logs.h"
> 
>  /* Mbox operation timeout in seconds */
>  #define MBOX_WAIT_TIME_SEC	3
> @@ -60,6 +59,17 @@ struct mbox_ram_hdr {
>  	};
>  };
> 
> +++ b/drivers/common/octeontx/octeontx_mbox.h
> @@ -6,10 +6,22 @@
>  #define __OCTEONTX_MBOX_H__
> 
>  #include <rte_common.h>
> +#include <rte_spinlock.h>
> 
>  #define SSOW_BAR4_LEN			(64 * 1024)
>  #define SSO_VHGRP_PF_MBOX(x)		(0x200ULL | ((x) << 3))
> 
> +#define MBOX_LOG(level, fmt, args...) \
> +	rte_log(RTE_LOG_ ## level, octeontx_logtype_mbox,\
> +			"%s() line %u: " fmt "\n", __func__, __LINE__, ## args)
> +
> +#define mbox_log_info(fmt, ...) MBOX_LOG(INFO, fmt, ##__VA_ARGS__)
> +#define mbox_log_dbg(fmt, ...) MBOX_LOG(DEBUG, fmt, ##__VA_ARGS__)
> +#define mbox_log_err(fmt, ...) MBOX_LOG(ERR, fmt, ##__VA_ARGS__)
> +#define mbox_func_trace mbox_log_dbg
> +
> +extern int octeontx_logtype_mbox;
> +
>  struct octeontx_ssovf_info {
>  	uint16_t domain; /* Domain id */
>  	uint8_t total_ssovfs; /* Total sso groups available in domain */
> @@ -30,6 +42,8 @@ struct octeontx_mbox_hdr {
> 
>  int octeontx_ssovf_info(struct octeontx_ssovf_info *info);
>  void *octeontx_ssovf_bar(enum octeontx_ssovf_type, uint8_t id, uint8_t bar);

IMO, prototype for octeontx_ssovf_bar(), octeontx_ssovf_info(),
and defintion octeontx_ssovf_info can be moved to driver/event/octeontx
as it is the only driver is using that(i.e no need not to be in common code)


> +int octeontx_mbox_set_ram_mbox_base(uint8_t *ram_mbox_base);
> +int octeontx_mbox_set_reg(uint8_t *reg);
>  int octeontx_ssovf_mbox_send(struct octeontx_mbox_hdr *hdr,
>  		void *txdata, uint16_t txlen, void *rxdata, uint16_t rxlen);
> 
> diff --git a/drivers/common/octeontx/rte_common_octeontx_version.map b/drivers/common/octeontx/rte_common_octeontx_version.map
> new file mode 100644
> index 000000000..59dbed5b2
> --- /dev/null
> +++ b/drivers/common/octeontx/rte_common_octeontx_version.map
> @@ -0,0 +1,9 @@
> +DPDK_18.05 {
> +	global:
> +
> +	octeontx_ssovf_info;
> +	octeontx_ssovf_bar;

Same as above, move above stuff from rte_common_octeontx_version.map

> +	octeontx_mbox_set_ram_mbox_base;
> +	octeontx_mbox_set_reg;
> +	octeontx_ssovf_mbox_send;

I think, octeontx_ssovf_mbox_send can be replaced with
octeontx_mbox_send() to inline with other APIs above(octeontx_mbox_set*)

> +};
> diff --git a/drivers/event/octeontx/Makefile b/drivers/event/octeontx/Makefile
> index 0e49efd84..34dcb844c 100644
> --- a/drivers/event/octeontx/Makefile
> +++ b/drivers/event/octeontx/Makefile
> @@ -10,10 +10,11 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  LIB = librte_pmd_octeontx_ssovf.a
> 
>  CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -I$(RTE_SDK)/drivers/common/octeontx/
>  CFLAGS += -I$(RTE_SDK)/drivers/mempool/octeontx/
>  CFLAGS += -I$(RTE_SDK)/drivers/net/octeontx/
> 
> -LDLIBS += -lrte_eal -lrte_eventdev -lrte_mempool_octeontx -lrte_pmd_octeontx
> +LDLIBS += -lrte_eal -lrte_eventdev -lrte_common_octeontx -lrte_pmd_octeontx
>  LDLIBS += -lrte_bus_pci -lrte_mempool -lrte_mbuf -lrte_kvargs
>  LDLIBS += -lrte_bus_vdev
> 
> @@ -27,6 +28,7 @@ LIBABIVER := 1
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += ssovf_worker.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += ssovf_evdev.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += ssovf_evdev_selftest.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += octeontx_ssovf.c

s/octeontx_ssovf.c/ssovf_probe.c

see next comment.

> 
>  ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>  CFLAGS_ssovf_worker.o += -fno-prefetch-loop-arrays
> diff --git a/drivers/event/octeontx/meson.build b/drivers/event/octeontx/meson.build
> index 358fc9fc9..1181f337b 100644
> --- a/drivers/event/octeontx/meson.build
> +++ b/drivers/event/octeontx/meson.build
> @@ -3,7 +3,8 @@
> 
>  sources = files('ssovf_worker.c',
>  		'ssovf_evdev.c',
> -		'ssovf_evdev_selftest.c'
> +		'ssovf_evdev_selftest.c',
> +		'octeontx_ssovf.c'

I think, it makes sense to change the name to ssovf_probe.c as all files
in this directory starts with ssovf_*

>  )
> 
> -deps += ['mempool_octeontx', 'bus_vdev', 'pmd_octeontx']
> +deps += ['common_octeontx', 'mempool_octeontx', 'bus_vdev', 'pmd_octeontx']
> diff --git a/drivers/mempool/octeontx/octeontx_ssovf.c b/drivers/event/octeontx/octeontx_ssovf.c
> similarity index 92%
> rename from drivers/mempool/octeontx/octeontx_ssovf.c
> rename to drivers/event/octeontx/octeontx_ssovf.c
> index 97b240665..c32b49a01 100644
> --- a/drivers/mempool/octeontx/octeontx_ssovf.c
> +++ b/drivers/event/octeontx/octeontx_ssovf.c
> @@ -10,7 +10,6 @@
>  #include <rte_bus_pci.h>
> 
>  #include "octeontx_mbox.h"
> -#include "octeontx_pool_logs.h"
> 
>  #define PCI_VENDOR_ID_CAVIUM              0x177D
>  #define PCI_DEVICE_ID_OCTEONTX_SSOGRP_VF  0xA04B
> @@ -142,6 +141,7 @@ ssowvf_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  	uint16_t vfid;
>  	struct ssowvf_res *res;
>  	struct ssowvf_identify *id;
> +	uint8_t *ram_mbox_base;
> 
>  	RTE_SET_USED(pci_drv);
> 
> @@ -180,6 +180,14 @@ ssowvf_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  	res->domain = id->domain;
> 
>  	sdev.total_ssowvfs++;
> +	if (!vfid) {

vfid == 0 ?

> +		ram_mbox_base = octeontx_ssovf_bar(OCTEONTX_SSO_HWS, 0, 4);
> +		if (octeontx_mbox_set_ram_mbox_base(ram_mbox_base)) {
> +			mbox_log_err("Invalid Failed to set ram mbox base");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	rte_wmb();
>  	mbox_log_dbg("Domain=%d hws=%d total_ssowvfs=%d", res->domain,
>  			res->vfid, sdev.total_ssowvfs);
> @@ -213,6 +221,7 @@ ssovf_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  	uint16_t vfid;
>  	uint8_t *idreg;
>  	struct ssovf_res *res;
> +	uint8_t *reg;
> 
>  	RTE_SET_USED(pci_drv);
> 
> @@ -246,6 +255,15 @@ ssovf_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
>  	res->domain = val & 0xffff;
> 
>  	sdev.total_ssovfs++;
> +	if (!vfid) {

vfid == 0 ?

> +		reg = octeontx_ssovf_bar(OCTEONTX_SSO_GROUP, 0, 0);
> +		reg += SSO_VHGRP_PF_MBOX(1);
> +		if (octeontx_mbox_set_reg(reg)) {
> +			mbox_log_err("Invalid Failed to set mbox_reg");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	rte_wmb();
>  	mbox_log_dbg("Domain=%d group=%d total_ssovfs=%d", res->domain,
>  			res->vfid, sdev.total_ssovfs);

With above changes:

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

  reply	other threads:[~2018-04-04  3:29 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-19  9:27 [dpdk-dev] [PATCH 1/2] drivers: add " Pavan Nikhilesh
2018-03-19  9:27 ` [dpdk-dev] [PATCH 2/2] octeontx: move mbox to " Pavan Nikhilesh
2018-03-19 10:35 ` [dpdk-dev] [PATCH 1/2] drivers: add " Shreyansh Jain
2018-03-20 14:38   ` Pavan Nikhilesh
2018-03-20 14:40 ` [dpdk-dev] [PATCH v2 " Pavan Nikhilesh
2018-03-20 14:40   ` [dpdk-dev] [PATCH v2 2/2] octeontx: move mbox to " Pavan Nikhilesh
2018-03-20 15:01     ` Hemant Agrawal
2018-03-20 16:00       ` Pavan Nikhilesh
2018-03-20 15:45   ` [dpdk-dev] [PATCH v2 1/2] drivers: add " Thomas Monjalon
2018-03-20 16:06     ` Pavan Nikhilesh
2018-03-20 17:01       ` Bruce Richardson
2018-03-20 17:27         ` Thomas Monjalon
2018-03-20 19:25           ` Trahe, Fiona
2018-03-26  7:53 ` [dpdk-dev] [PATCH v3 " Pavan Nikhilesh
2018-03-26  7:53   ` [dpdk-dev] [PATCH v3 2/2] octeontx: move mbox to " Pavan Nikhilesh
2018-03-27 16:15     ` Thomas Monjalon
2018-03-30 17:23       ` Pavan Nikhilesh
2018-03-27 16:11   ` [dpdk-dev] [PATCH v3 1/2] drivers: add " Thomas Monjalon
2018-04-02  9:09 ` [dpdk-dev] [PATCH v4 " Pavan Nikhilesh
2018-04-02  9:10   ` [dpdk-dev] [PATCH v4 2/2] octeontx: move mbox to " Pavan Nikhilesh
2018-04-04  3:29     ` Jerin Jacob [this message]
2018-04-04  5:06       ` santosh
2018-04-04  9:23     ` Thomas Monjalon
2018-04-03  6:57   ` [dpdk-dev] [PATCH v4 1/2] drivers: add " Hemant Agrawal
2018-04-04  3:08     ` Jerin Jacob
2018-04-04  5:01   ` santosh
2018-04-04 14:30 ` [dpdk-dev] [PATCH v5 " Pavan Nikhilesh
2018-04-04 14:30   ` [dpdk-dev] [PATCH v5 2/2] octeontx: move mbox to " Pavan Nikhilesh
2018-04-04 14:37     ` Thomas Monjalon
2018-04-04 21:20     ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180404032919.GB8574@jerin \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=anatoly.burakov@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=fiona.trahe@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=lironh@marvell.com \
    --cc=pbhagavatula@caviumnetworks.com \
    --cc=santosh.shukla@caviumnetworks.com \
    --cc=shreyansh.jain@nxp.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).