> -----Original Message-----
> From: Laurentiu Tudor
> Sent: Friday, December 02, 2016 6:12 AM
> To: Stuart Yoder <stuart.yo...@nxp.com>; gre...@linuxfoundation.org
> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org; ag...@suse.de;
> a...@arndb.de; Leo Li
> <leoyang...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>; Catalin Horghidan
> <catalin.horghi...@nxp.com>; Ruxandra Ioana Radulescu
> <ruxandra.radule...@nxp.com>; Roy Pledge
> <roy.ple...@nxp.com>
> Subject: Re: [PATCH v3 4/9] bus: fsl-mc: dpio: add frame descriptor and
> scatter/gather APIs
>
>
> Some more bits and pieces inside.
>
> ---
> Best Regards, Laurentiu
>
> On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> > From: Roy Pledge <roy.ple...@nxp.com>
> >
> > Add global definitions for DPAA2 frame descriptors and scatter
> > gather entries.
> >
> > Signed-off-by: Roy Pledge <roy.ple...@nxp.com>
> > Signed-off-by: Stuart Yoder <stuart.yo...@nxp.com>
> > ---
> >
> > Notes:
> > -v3
> > -no changes
> > -v2
> > -added setter/getter for the FD ctrl field
> > -corrected comment for SG format_offset field description
> > -added support for short length field in FD
> >
> > include/linux/fsl/dpaa2-fd.h | 448
> > +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 448 insertions(+)
> > create mode 100644 include/linux/fsl/dpaa2-fd.h
> >
> > diff --git a/include/linux/fsl/dpaa2-fd.h b/include/linux/fsl/dpaa2-fd.h
> > new file mode 100644
> > index 0000000..182c8f4
> > --- /dev/null
> > +++ b/include/linux/fsl/dpaa2-fd.h
> > @@ -0,0 +1,448 @@
> > +/*
> > + * Copyright 2014-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 Freescale Semiconductor nor the
> > + * names of its 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 Freescale Semiconductor ``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 Freescale Semiconductor 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_DPAA2_FD_H
> > +#define __FSL_DPAA2_FD_H
> > +
> > +#include <linux/kernel.h>
> > +
> > +/**
> > + * DOC: DPAA2 FD - Frame Descriptor APIs for DPAA2
> > + *
> > + * Frame Descriptors (FDs) are used to describe frame data in the DPAA2.
> > + * Frames can be enqueued and dequeued to Frame Queues (FQs) which are
> > consumed
> > + * by the various DPAA accelerators (WRIOP, SEC, PME, DCE)
> > + *
> > + * There are three types of frames: single, scatter gather, and frame
> > lists.
> > + *
> > + * The set of APIs in this file must be used to create, manipulate and
> > + * query Frame Descriptors.
> > + */
> > +
> > +/**
> > + * struct dpaa2_fd - Struct describing FDs
> > + * @words: for easier/faster copying the whole FD structure
> > + * @addr: address in the FD
> > + * @len: length in the FD
> > + * @bpid: buffer pool ID
> > + * @format_offset: format, offset, and short-length fields
> > + * @frc: frame context
> > + * @ctrl: control bits...including dd, sc, va, err, etc
> > + * @flc: flow context address
> > + *
> > + * This structure represents the basic Frame Descriptor used in the system.
> > + */
> > +struct dpaa2_fd {
> > + union {
> > + u32 words[8];
> > + struct dpaa2_fd_simple {
> > + __le64 addr;
> > + __le32 len;
> > + __le16 bpid;
> > + __le16 format_offset;
> > + __le32 frc;
> > + __le32 ctrl;
> > + __le64 flc;
> > + } simple;
> > + };
> > +};
> > +
> > +#define FD_SHORT_LEN_FLAG_MASK 0x1
> > +#define FD_SHORT_LEN_FLAG_SHIFT 14
> > +#define FD_SHORT_LEN_MASK 0x1FFFF
> > +#define FD_OFFSET_MASK 0x0FFF
> > +#define FD_FORMAT_MASK 0x3
> > +#define FD_FORMAT_SHIFT 12
> > +#define SG_SHORT_LEN_FLAG_MASK 0x1
> > +#define SG_SHORT_LEN_FLAG_SHIFT 14
> > +#define SG_SHORT_LEN_MASK 0x1FFFF
> > +#define SG_OFFSET_MASK 0x0FFF
> > +#define SG_FORMAT_MASK 0x3
> > +#define SG_FORMAT_SHIFT 12
> > +#define SG_BPID_MASK 0x3FFF
> > +#define SG_FINAL_FLAG_MASK 0x1
> > +#define SG_FINAL_FLAG_SHIFT 15
>
> We should align the values of these macros as we do in other places.
Yes, agree.
> > +enum dpaa2_fd_format {
> > + dpaa2_fd_single = 0,
> > + dpaa2_fd_list,
> > + dpaa2_fd_sg
> > +};
> > +
> > +/**
> > + * dpaa2_fd_get_addr() - get the addr field of frame descriptor
> > + * @fd: the given frame descriptor
> > + *
> > + * Return the address in the frame descriptor.
> > + */
> > +static inline dma_addr_t dpaa2_fd_get_addr(const struct dpaa2_fd *fd)
> > +{
> > +
>
> Extra empty line.
Ok.
> > +/**
> > + * dpaa2_fd_set_offset() - Set the offset field of frame descriptor
> > + * @fd: the given frame descriptor
> > + * @offset: the offset needs to be set in frame descriptor
> > + */
> > +static inline void dpaa2_fd_set_offset(struct dpaa2_fd *fd, uint16_t
> > offset)
> > +{
> > + fd->simple.format_offset &= ~(FD_OFFSET_MASK);
>
> Paranthesis not needed.
Ok.
> > +/**
> > + * dpaa2_sg_set_offset() - Set the offset in SG entry
> > + * @sg: the given scatter-gathering object
> > + * @offset: the offset to be set
> > + */
> > +static inline void dpaa2_sg_set_offset(struct dpaa2_sg_entry *sg,
> > + u16 offset)
> > +{
> > + sg->format_offset &= cpu_to_le16(~(SG_OFFSET_MASK));
>
> Extra-paranthesis around SG_OFFSET_MASK not needed.
Ok.
> > +/**
> > + * dpaa2_sg_is_final() - Check final bit in SG entry
> > + * @sg: the given scatter-gathering object
> > + *
> > + * Return bool.
> > + */
> > +static inline bool dpaa2_sg_is_final(const struct dpaa2_sg_entry *sg)
> > +{
> > + return !!(le16_to_cpu(sg->format_offset) >> SG_FINAL_FLAG_SHIFT);
>
> In other places in this file we don't use the '!!' to convert to bool. Maybe
> we should drop it here too.
As per the separate comments with Dan, I'll leave and check for consistency
with other places in this file.
Thanks,
Stuart
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
