On 13-06-19, 10:13, Peng Ma wrote:
> The MC(Management Complex) exports the DPDMAI(Data Path DMA Interface)
> object as an interface to operate the DPAA2(Data Path Acceleration
> Architecture 2) qDMA Engine. The DPDMAI enables sending frame-based
> requests to qDMA and receiving back confirmation response on transaction
> completion, utilizing the DPAA2 QBMan(Queue Manager and Buffer Manager
> hardware) infrastructure. DPDMAI object provides up to two priorities for
> processing qDMA requests.
> The following list summarizes the DPDMAI main features and capabilities:
> 1. Supports up to two scheduling priorities for processing
> service requests.
> - Each DPDMAI transmit queue is mapped to one of two service
> priorities, allowing further prioritization in hardware between
> requests from different DPDMAI objects.
> 2. Supports up to two receive queues for incoming transaction
> completion confirmations.
> - Each DPDMAI receive queue is mapped to one of two receive
> priorities, allowing further prioritization between other
> interfaces when associating the DPDMAI receive queues to DPIO
> or DPCON(Data Path Concentrator) objects.
> 3. Supports different scheduling options for processing received
> packets:
> - Queues can be configured either in 'parked' mode (default),
> oattached to a DPIO object, or attached to DPCON object.
^^^^^^^^^
typo?
> +struct dpdmai_cmd_open {
> + __le32 dpdmai_id;
> +};
Do you really need a struct to handle a 32bit value?
And seeing it used once, we can skip and avoid needless cast in usage as
well!
> +/* cmd, param, offset, width, type, arg_name */
> +#define DPDMAI_CMD_CREATE(_cmd, _cfg) \
> +do { \
> + typeof(_cmd) (cmd) = (_cmd); \
> + typeof(_cfg) (cfg) = (_cfg); \
> + MC_CMD_OP(cmd, 0, 8, 8, u8, (cfg)->priorities[0]);\
> + MC_CMD_OP(cmd, 0, 16, 8, u8, (cfg)->priorities[1]);\
> +} while (0)
> +
> +static inline u64 mc_enc(int lsoffset, int width, u64 val)
> +{
> + return (u64)(((u64)val & MAKE_UMASK64(width)) << lsoffset);
this looks not so nice. val is u64 so why is it required to cast to u64
again?
> +}
> +
> +static inline u64 mc_dec(u64 val, int lsoffset, int width)
> +{
> + return (u64)((val >> lsoffset) & MAKE_UMASK64(width));
do we need the cast here?
> +int dpdmai_create(struct fsl_mc_io *mc_io, u32 cmd_flags,
> + const struct dpdmai_cfg *cfg, u16 *token)
> +{
> + struct fsl_mc_command cmd = { 0 };
> + int err;
> +
> + /* prepare command */
> + cmd.header = mc_encode_cmd_header(DPDMAI_CMDID_CREATE,
> + cmd_flags,
> + 0);
This seems to fit in a single line!
> + DPDMAI_CMD_CREATE(cmd, cfg);
> +
> + /* 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.header);
This looks very similar to dpdmai_open() and I suppose you can create a
macro to create and send cmd with optional params!
> +
> + return 0;
> +}
> +
> +int dpdmai_enable(struct fsl_mc_io *mc_io, u32 cmd_flags,
> + u16 token)
> +{
> + struct fsl_mc_command cmd = { 0 };
> +
> + /* prepare command */
> + cmd.header = mc_encode_cmd_header(DPDMAI_CMDID_ENABLE,
> + cmd_flags,
> + token);
This can fit in two lines
> +
> + /* send command to mc*/
> + return mc_send_command(mc_io, &cmd);
> +}
> +
> +int dpdmai_disable(struct fsl_mc_io *mc_io, u32 cmd_flags,
> + u16 token)
why two lines for this!
> +{
> + struct fsl_mc_command cmd = { 0 };
> +
> + /* prepare command */
> + cmd.header = mc_encode_cmd_header(DPDMAI_CMDID_DISABLE,
> + cmd_flags,
> + token);
This also!
Please check rest of the driver for these style issues and see bunch of
places can fit into a line or two!
> +/**
> + * dpdmai_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_'
> + * @dpdmai_id: DPDMAI 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 dpdmai_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.
This is good but can you move them to C file with the function
--
~Vinod
