Date:   Fri, 2 Dec 2016 11:26:46 +0000
From:   Laurentiu Tudor <laurentiu.tudor@....com>
To:     Stuart Yoder <stuart.yoder@....com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "agraf@...e.de" <agraf@...e.de>, "arnd@...db.de" <arnd@...db.de>,
        Leo Li <leoyang.li@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        "Catalin Horghidan" <catalin.horghidan@....com>,
        Ruxandra Ioana Radulescu <ruxandra.radulescu@....com>,
        Roy Pledge <roy.pledge@....com>
Subject: Re: [PATCH v3 3/9] bus: fsl-mc: dpio: add APIs for DPIO objects

Couple of nits inline.

---
Best Regards, Laurentiu

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Ioana Radulescu <ruxandra.radulescu@....com>
> 
> Add the command build/parse APIs for operating on DPIO objects through
> the DPAA2 Management Complex.
> 
> Signed-off-by: Ioana Radulescu <ruxandra.radulescu@....com>
> Signed-off-by: Roy Pledge <roy.pledge@....com>
> Signed-off-by: Stuart Yoder <stuart.yoder@....com>
> ---
> 
> Notes:
>     -v3
>       -no changes
>     -v2
>       -removed unused structs and defines
> 
>  drivers/bus/fsl-mc/Kconfig         |  10 ++
>  drivers/bus/fsl-mc/Makefile        |   3 +
>  drivers/bus/fsl-mc/dpio/Makefile   |   9 ++
>  drivers/bus/fsl-mc/dpio/dpio-cmd.h |  75 ++++++++++++
>  drivers/bus/fsl-mc/dpio/dpio.c     | 229 +++++++++++++++++++++++++++++++++++++
>  drivers/bus/fsl-mc/dpio/dpio.h     | 108 +++++++++++++++++
>  6 files changed, 434 insertions(+)
>  create mode 100644 drivers/bus/fsl-mc/dpio/Makefile
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio-cmd.h
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio.c
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio.h
> 
> diff --git a/drivers/bus/fsl-mc/Kconfig b/drivers/bus/fsl-mc/Kconfig
> index 5c009ab..a10aaf0 100644
> --- a/drivers/bus/fsl-mc/Kconfig
> +++ b/drivers/bus/fsl-mc/Kconfig
> @@ -15,3 +15,13 @@ config FSL_MC_BUS
>  	  architecture.  The fsl-mc bus driver handles discovery of
>  	  DPAA2 objects (which are represented as Linux devices) and
>  	  binding objects to drivers.
> +
> +config FSL_MC_DPIO
> +        tristate "QorIQ DPAA2 DPIO driver"
> +        depends on FSL_MC_BUS
> +        help
> +	  Driver for the DPAA2 DPIO object.  A DPIO provides queue and
> +	  buffer management facilities for software to interact with
> +	  other DPAA2 objects. This driver does not expose the DPIO
> +	  objects individually, but groups them under a service layer
> +	  API.
> diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
> index d56afee..d18df72 100644
> --- a/drivers/bus/fsl-mc/Makefile
> +++ b/drivers/bus/fsl-mc/Makefile
> @@ -17,3 +17,6 @@ mc-bus-driver-objs := fsl-mc-bus.o \
>  		      fsl-mc-msi.o \
>  		      dpmcp.o \
>  		      dpbp.o
> +
> +# MC DPIO driver
> +obj-$(CONFIG_FSL_MC_DPIO) += dpio/
> diff --git a/drivers/bus/fsl-mc/dpio/Makefile b/drivers/bus/fsl-mc/dpio/Makefile
> new file mode 100644
> index 0000000..128befc
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/Makefile
> @@ -0,0 +1,9 @@
> +#
> +# QorIQ DPAA2 DPIO driver
> +#
> +
> +subdir-ccflags-y := -Werror
> +
> +obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
> +
> +fsl-mc-dpio-objs := dpio.o
> diff --git a/drivers/bus/fsl-mc/dpio/dpio-cmd.h b/drivers/bus/fsl-mc/dpio/dpio-cmd.h
> new file mode 100644
> index 0000000..3464ed9
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/dpio-cmd.h
> @@ -0,0 +1,75 @@
> +/*
> + * Copyright 2013-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + * names of any contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +#ifndef _FSL_DPIO_CMD_H
> +#define _FSL_DPIO_CMD_H
> +
> +/* DPIO Version */
> +#define DPIO_VER_MAJOR			4
> +#define DPIO_VER_MINOR			2
> +
> +/* Command Versioning */
> +
> +#define DPIO_CMD_ID_OFFSET		4
> +#define DPIO_CMD_BASE_VERSION		1
> +
> +#define DPIO_CMD(id)	((id << DPIO_CMD_ID_OFFSET) | DPIO_CMD_BASE_VERSION)

Paranthesis around 'id'?

> +/* Command IDs */
> +#define DPIO_CMDID_CLOSE				DPIO_CMD(0x800)
> +#define DPIO_CMDID_OPEN					DPIO_CMD(0x803)
> +#define DPIO_CMDID_GET_API_VERSION			DPIO_CMD(0xa03)
> +#define DPIO_CMDID_ENABLE				DPIO_CMD(0x002)
> +#define DPIO_CMDID_DISABLE				DPIO_CMD(0x003)
> +#define DPIO_CMDID_GET_ATTR				DPIO_CMD(0x004)
> +
> +struct dpio_cmd_open {
> +	__le32 dpio_id;
> +};
> +
> +#define DPIO_CHANNEL_MODE_MASK		0x3
> +
> +struct dpio_rsp_get_attr {
> +	/* cmd word 0 */
> +	__le32 id;
> +	__le16 qbman_portal_id;
> +	u8 num_priorities;
> +	u8 channel_mode;
> +	/* cmd word 1 */
> +	__le64 qbman_portal_ce_addr;
> +	/* cmd word 2 */
> +	__le64 qbman_portal_ci_addr;
> +	/* cmd word 3 */
> +	__le32 pad;
> +	__le32 qbman_version;
> +};
> +
> +#endif /* _FSL_DPIO_CMD_H */
> diff --git a/drivers/bus/fsl-mc/dpio/dpio.c b/drivers/bus/fsl-mc/dpio/dpio.c
> new file mode 100644
> index 0000000..5f1ee19
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/dpio.c
> @@ -0,0 +1,229 @@
> +/*
> + * Copyright 2013-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + * names of any contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +#include <linux/fsl/mc-sys.h>
> +#include <linux/fsl/mc-cmd.h>
> +
> +#include "dpio.h"
> +#include "dpio-cmd.h"
> +
> +/*
> + * Data Path I/O Portal API
> + * Contains initialization APIs and runtime control APIs for DPIO
> + */
> +
> +/**
> + * dpio_open() - Open a control session for the specified object
> + * @mc_io:	Pointer to MC portal's I/O object
> + * @cmd_flags:	Command flags; one or more of 'MC_CMD_FLAG_'
> + * @dpio_id:	DPIO unique ID
> + * @token:	Returned token; use in subsequent API calls
> + *
> + * This function can be used to open a control session for an
> + * already created object; an object may have been declared in
> + * the DPL or by calling the dpio_create() function.
> + * This function returns a unique authentication token,
> + * associated with the specific object ID and the specific MC
> + * portal; this token must be used in all subsequent commands for
> + * this specific object.
> + *
> + * Return:	'0' on Success; Error code otherwise.
> + */
> +int dpio_open(struct fsl_mc_io *mc_io,
> +	      u32 cmd_flags,
> +	      int dpio_id,
> +	      u16 *token)
> +{
> +	struct mc_command cmd = { 0 };
> +	struct dpio_cmd_open *dpio_cmd;
> +	int err;
> +
> +	/* prepare command */
> +	cmd.header = mc_encode_cmd_header(DPIO_CMDID_OPEN,
> +					  cmd_flags,
> +					  0);
> +	dpio_cmd = (struct dpio_cmd_open *)cmd.params;
> +	dpio_cmd->dpio_id = cpu_to_le32(dpio_id);
> +
> +	/* send command to mc*/
> +	err = mc_send_command(mc_io, &cmd);
> +	if (err)
> +		return err;
> +
> +	/* retrieve response parameters */
> +	*token = mc_cmd_hdr_read_token(&cmd);

Nit: maybe we should drop these repetitive "prepare / send / retrieve" comments
as the code is pretty self explanatory.

---
Best Regards, Laurentiu

Powered by blists - more mailing lists